memcpy() changes memory which should not be touched

Question

In my code I use array addressing on a heap pointer int *shifts to save values into this array. When I use memcpy() to copy data between heap areas, the content of the array shifts[y] is changed, even I don't see the possibility of a buffer overflow. The problem already happens in the first round of the for loop with s->start set to 568.

The code snippet is part of the FFmpeg library. av_malloc() and av_log() are wrappers on malloc() and printf().

I've tried to output the memory addresses in hope to find out the reason for the problem. Please excuse the bad readability because of the print out code to demonstrate the values after each step.

typedef struct LineShiftContext {
    const AVClass *class;
    /* Command line options: */
    int lines, start;
    /* Internal: */
    int nb_planes;
    int planewidth[4];
    int planeheight[4];
    void *filler[4];
    int *shifts;

    void (*recoverlineshifts)(struct LineShiftContext *s, AVFrame *frame);
} LineShiftContext;
////////////////
    for (int p = 0; p < s->nb_planes; p++) {
        s->filler[p] = av_malloc(s->planewidth[p] * (depth <= 8 ? sizeof(uint8_t) : sizeof(uint16_t)));
        av_log(NULL, AV_LOG_ERROR, "s->planewidth[%d]: %d, s->filler[%d]: %p\n", p, s->planewidth[p], p, s->filler[p]);
    }
    s->shifts = av_malloc(s->planeheight[0]);
    av_log(NULL, AV_LOG_ERROR, "s->planeheight[0]: %d, s->shifts:   %p\n", s->planeheight[0], s->shifts);
////////////////
static void recoverlineshifts(LineShiftContext *s, AVFrame *frame)
{
    uint8_t *data = frame->data[0];
    int16_t lz = frame->linesize[0];
    int width = s->planewidth[0];
    int height = s->planeheight[0];
    int increment = s->lines >= 0 ? 1 : -1; // if negative, run upwards
    uint8_t *temp = (uint8_t *)s->filler[0];
    int *shifts = s->shifts;
    memset(shifts, 0, height);

    for (int y = s->start, shift_old = 0; y != s->start + s->lines; y += increment) {
        shifts[y] = calculate_shift(s, data + (y - increment) * lz, shift_old, width, data + y * lz);

        av_log(NULL, AV_LOG_ERROR, "temp:   %p, line:      %p, lineref: %p, lz: %d, y: %d, yref: %d, shifts: %p, shifts[y]: %d\n",
                temp, data + y * lz, data + (y - increment) * lz, lz, y, y - increment, shifts, shifts[y]);
        av_log(NULL, AV_LOG_ERROR, "temp++: %p, line++:    %p, width: %d\n",
                temp + FFMAX(0, -shifts[y]), data + y * lz + FFMAX(0, shifts[y]), width - abs(shifts[y]));
        av_log(NULL, AV_LOG_ERROR, "shifts: %p, y: %d, shifts[y]: %d, -shifts[y]: %d, FFMAX(0, -shifts[y]): %d\n",
                shifts, y, shifts[y], -shifts[y], FFMAX(0, -shifts[y]));
        memcpy(temp + FFMAX(0, -shifts[y]), data + y * lz + FFMAX(0, shifts[y]), width - abs(shifts[y]));
        av_log(NULL, AV_LOG_ERROR, "shifts: %p, y: %d, shifts[y]: %d, -shifts[y]: %d, FFMAX(0, -shifts[y]): %d\n",
                shifts, y, shifts[y], -shifts[y], FFMAX(0, -shifts[y]));
        ////////////
    }
}
////////////////
    for (int p = 0; p < s->nb_planes; p++)
        av_freep(&s->filler[p]);
    av_freep(&s->shifts);

Result:

s->planewidth[0]: 704, s->filler[0]: 0x55f36e025940
s->planewidth[1]: 352, s->filler[1]: 0x55f36e025c80
s->planewidth[2]: 352, s->filler[2]: 0x55f36e024a80
s->planeheight[0]: 576, s->shifts:   0x55f36e0251c0
temp:   0x55f36e025940, line:      0x7f8f0e06aa40, lineref: 0x7f8f0e06a780, lz: 704, y: 568, yref: 567, shifts: 0x55f36e0251c0, shifts[y]: -12
temp++: 0x55f36e02594c, line++:    0x7f8f0e06aa40, width: 692
shifts: 0x55f36e0251c0, y: 568, shifts[y]: -12, -shifts[y]: 12, FFMAX(0, -shifts[y]): 12
shifts: 0x55f36e0251c0, y: 568, shifts[y]: 134678279, -shifts[y]: -134678279, FFMAX(0, -shifts[y]): 0

Expected (for the last line):

shifts: 0x55f36e0251c0, y: 568, shifts[y]: -12, -shifts[y]: 12, FFMAX(0, -shifts[y]): 12

Later in the for loop I have:

            av_log(NULL, AV_LOG_ERROR, "temp  : %p, lineref:   %p, width: %d\n", temp, data + (y - increment) * lz, FFMAX(0, -shifts[y]));
            memcpy(temp, data + (y - increment) * lz,
                    FFMAX(0, -shifts[y])); // fill left gap from reference line
            av_log(NULL, AV_LOG_ERROR, "temp++: %p, lineref++: %p, width: %d\n", temp + width - FFMAX(0, shifts[y]), data + (y - increment) * lz + width - FFMAX(0, shifts[y]), FFMAX(0, shifts[y]));
            memcpy(temp + width - FFMAX(0, shifts[y]), data + (y - increment) * lz + width - FFMAX(0, shifts[y]),
                    FFMAX(0, shifts[y])); // fill right gap from reference line
            av_log(NULL, AV_LOG_ERROR, "line:   %p, temp:      %p, width: %d\n", data + y * lz, temp, width);
            memcpy(data + y * lz, temp, width);

where even the pointers run out of scope:

temp  : 0x55f36e025940, lineref:   0x7f8f0e06a780, width: 0
temp++: 0x55f365fb54f9, lineref++: 0x7f8f05ffa339, width: 134678279

... which finally results in a memory access error.

Answer 1

The problem here is that shifts is being under-allocated and under-initialized. The definition if shifts is:

int *shifts;

Its allocation and initialization are:

s->shifts = av_malloc(s->planeheight[0]);

int height = s->planeheight[0];
int *shifts = s->shifts;
memset(shifts, 0, height);

Each of these are missing a multiplication by sizeof(int). It should be:

s->shifts = av_malloc(s->planeheight[0] * sizeof(int));

int *shifts = s->shifts;
memset(shifts, 0, height * sizeof(int));