This patch fixes possible race conditions in queue management with SMP:
when a frame was completed, the irq function tried to use the next frame
buffer. At this time, it was possible that the application on an other
processor updated the frame pointer, making the image to point to a bad
buffer.
The patch contains two main changes:
- the image transfer uses the queue indexes which are protected against
simultaneous memory access,
- the image pointer which is used for image concatenation is only set at
interrupt level.
Some subdrivers which used the image pointer have been updated.
Reported-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Jean-François Moine <moinejf@free.fr>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
atomic_set(&sd->cam_exposure, data[39] * 2);
atomic_set(&sd->fps, data[41]);
atomic_set(&sd->cam_exposure, data[39] * 2);
atomic_set(&sd->fps, data[41]);
- image = gspca_dev->image;
- if (image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
-
/* Check for proper EOF for last frame */
/* Check for proper EOF for last frame */
- if (gspca_dev->image_len > 4 &&
+ image = gspca_dev->image;
+ if (image != NULL &&
+ gspca_dev->image_len > 4 &&
image[gspca_dev->image_len - 4] == 0xff &&
image[gspca_dev->image_len - 3] == 0xff &&
image[gspca_dev->image_len - 2] == 0xff &&
image[gspca_dev->image_len - 4] == 0xff &&
image[gspca_dev->image_len - 3] == 0xff &&
image[gspca_dev->image_len - 2] == 0xff &&
urb->status = 0;
goto resubmit;
}
urb->status = 0;
goto resubmit;
}
- if (gspca_dev->image == NULL)
- gspca_dev->last_packet_type = DISCARD_PACKET;
pkt_scan = gspca_dev->sd_desc->pkt_scan;
for (i = 0; i < urb->number_of_packets; i++) {
pkt_scan = gspca_dev->sd_desc->pkt_scan;
for (i = 0; i < urb->number_of_packets; i++) {
PDEBUG(D_PACK, "add t:%d l:%d", packet_type, len);
PDEBUG(D_PACK, "add t:%d l:%d", packet_type, len);
- /* check the availability of the frame buffer */
- if (gspca_dev->image == NULL)
- return;
-
if (packet_type == FIRST_PACKET) {
if (packet_type == FIRST_PACKET) {
+ i = atomic_read(&gspca_dev->fr_i);
+
+ /* if there are no queued buffer, discard the whole frame */
+ if (i == atomic_read(&gspca_dev->fr_q)) {
+ gspca_dev->last_packet_type = DISCARD_PACKET;
+ return;
+ }
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
frame->v4l2_buf.timestamp = ktime_to_timeval(ktime_get());
frame->v4l2_buf.sequence = ++gspca_dev->sequence;
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
frame->v4l2_buf.timestamp = ktime_to_timeval(ktime_get());
frame->v4l2_buf.sequence = ++gspca_dev->sequence;
+ gspca_dev->image = frame->data;
gspca_dev->image_len = 0;
} else if (gspca_dev->last_packet_type == DISCARD_PACKET) {
if (packet_type == LAST_PACKET)
gspca_dev->image_len = 0;
} else if (gspca_dev->last_packet_type == DISCARD_PACKET) {
if (packet_type == LAST_PACKET)
}
gspca_dev->last_packet_type = packet_type;
}
gspca_dev->last_packet_type = packet_type;
- /* if last packet, wake up the application and advance in the queue */
+ /* if last packet, invalidate packet concatenation until
+ * next first packet, wake up the application and advance
+ * in the queue */
if (packet_type == LAST_PACKET) {
if (packet_type == LAST_PACKET) {
+ i = atomic_read(&gspca_dev->fr_i);
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
frame->v4l2_buf.bytesused = gspca_dev->image_len;
frame->v4l2_buf.flags = (frame->v4l2_buf.flags
| V4L2_BUF_FLAG_DONE)
& ~V4L2_BUF_FLAG_QUEUED;
j = gspca_dev->fr_queue[i];
frame = &gspca_dev->frame[j];
frame->v4l2_buf.bytesused = gspca_dev->image_len;
frame->v4l2_buf.flags = (frame->v4l2_buf.flags
| V4L2_BUF_FLAG_DONE)
& ~V4L2_BUF_FLAG_QUEUED;
+ i = (i + 1) % GSPCA_MAX_FRAMES;
+ atomic_set(&gspca_dev->fr_i, i);
wake_up_interruptible(&gspca_dev->wq); /* event = new frame */
wake_up_interruptible(&gspca_dev->wq); /* event = new frame */
- i = (i + 1) % gspca_dev->nframes;
- gspca_dev->fr_i = i;
- PDEBUG(D_FRAM, "frame complete len:%d q:%d i:%d o:%d",
- frame->v4l2_buf.bytesused,
- gspca_dev->fr_q,
- i,
- gspca_dev->fr_o);
- j = gspca_dev->fr_queue[i];
- frame = &gspca_dev->frame[j];
- if ((frame->v4l2_buf.flags & BUF_ALL_FLAGS)
- == V4L2_BUF_FLAG_QUEUED) {
- gspca_dev->image = frame->data;
- } else {
- gspca_dev->image = NULL;
- }
+ PDEBUG(D_FRAM, "frame complete len:%d",
+ frame->v4l2_buf.bytesused);
+ gspca_dev->image = NULL;
+ gspca_dev->image_len = 0;
}
}
EXPORT_SYMBOL(gspca_frame_add);
}
}
EXPORT_SYMBOL(gspca_frame_add);
PDEBUG(D_STREAM, "frame alloc frsz: %d", frsz);
frsz = PAGE_ALIGN(frsz);
gspca_dev->frsz = frsz;
PDEBUG(D_STREAM, "frame alloc frsz: %d", frsz);
frsz = PAGE_ALIGN(frsz);
gspca_dev->frsz = frsz;
- if (count > GSPCA_MAX_FRAMES)
- count = GSPCA_MAX_FRAMES;
+ if (count >= GSPCA_MAX_FRAMES)
+ count = GSPCA_MAX_FRAMES - 1;
gspca_dev->frbuf = vmalloc_32(frsz * count);
if (!gspca_dev->frbuf) {
err("frame alloc failed");
gspca_dev->frbuf = vmalloc_32(frsz * count);
if (!gspca_dev->frbuf) {
err("frame alloc failed");
frame->data = gspca_dev->frbuf + i * frsz;
frame->v4l2_buf.m.offset = i * frsz;
}
frame->data = gspca_dev->frbuf + i * frsz;
frame->v4l2_buf.m.offset = i * frsz;
}
- gspca_dev->fr_i = gspca_dev->fr_o = gspca_dev->fr_q = 0;
- gspca_dev->image = NULL;
- gspca_dev->image_len = 0;
- gspca_dev->last_packet_type = DISCARD_PACKET;
- gspca_dev->sequence = 0;
+ atomic_set(&gspca_dev->fr_q, 0);
+ atomic_set(&gspca_dev->fr_i, 0);
+ gspca_dev->fr_o = 0;
+ /* reset the streaming variables */
+ gspca_dev->image = NULL;
+ gspca_dev->image_len = 0;
+ gspca_dev->last_packet_type = DISCARD_PACKET;
+ gspca_dev->sequence = 0;
+
gspca_dev->usb_err = 0;
/* set the higher alternate setting and
gspca_dev->usb_err = 0;
/* set the higher alternate setting and
enum v4l2_buf_type buf_type)
{
struct gspca_dev *gspca_dev = priv;
enum v4l2_buf_type buf_type)
{
struct gspca_dev *gspca_dev = priv;
if (buf_type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
if (buf_type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
return -EINVAL;
gspca_stream_off(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock);
gspca_stream_off(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock);
- /* empty the application queues */
- for (i = 0; i < gspca_dev->nframes; i++)
- gspca_dev->frame[i].v4l2_buf.flags &= ~BUF_ALL_FLAGS;
- gspca_dev->fr_i = gspca_dev->fr_o = gspca_dev->fr_q = 0;
- gspca_dev->last_packet_type = DISCARD_PACKET;
- gspca_dev->sequence = 0;
+ /* empty the transfer queues */
+ atomic_set(&gspca_dev->fr_q, 0);
+ atomic_set(&gspca_dev->fr_i, 0);
+ gspca_dev->fr_o = 0;
ret = 0;
out:
mutex_unlock(&gspca_dev->queue_lock);
ret = 0;
out:
mutex_unlock(&gspca_dev->queue_lock);
int n;
n = parm->parm.capture.readbuffers;
int n;
n = parm->parm.capture.readbuffers;
- if (n == 0 || n > GSPCA_MAX_FRAMES)
+ if (n == 0 || n >= GSPCA_MAX_FRAMES)
parm->parm.capture.readbuffers = gspca_dev->nbufread;
else
gspca_dev->nbufread = n;
parm->parm.capture.readbuffers = gspca_dev->nbufread;
else
gspca_dev->nbufread = n;
static int frame_wait(struct gspca_dev *gspca_dev,
int nonblock_ing)
{
static int frame_wait(struct gspca_dev *gspca_dev,
int nonblock_ing)
{
- struct gspca_frame *frame;
- int i, j, ret;
/* check if a frame is ready */
i = gspca_dev->fr_o;
/* check if a frame is ready */
i = gspca_dev->fr_o;
- j = gspca_dev->fr_queue[i];
- frame = &gspca_dev->frame[j];
-
- if (!(frame->v4l2_buf.flags & V4L2_BUF_FLAG_DONE)) {
+ if (i == atomic_read(&gspca_dev->fr_i)) {
if (nonblock_ing)
return -EAGAIN;
/* wait till a frame is ready */
ret = wait_event_interruptible_timeout(gspca_dev->wq,
if (nonblock_ing)
return -EAGAIN;
/* wait till a frame is ready */
ret = wait_event_interruptible_timeout(gspca_dev->wq,
- (frame->v4l2_buf.flags & V4L2_BUF_FLAG_DONE) ||
+ i != atomic_read(&gspca_dev->fr_i) ||
!gspca_dev->streaming || !gspca_dev->present,
msecs_to_jiffies(3000));
if (ret < 0)
!gspca_dev->streaming || !gspca_dev->present,
msecs_to_jiffies(3000));
if (ret < 0)
- gspca_dev->fr_o = (i + 1) % gspca_dev->nframes;
- PDEBUG(D_FRAM, "frame wait q:%d i:%d o:%d",
- gspca_dev->fr_q,
- gspca_dev->fr_i,
- gspca_dev->fr_o);
+ gspca_dev->fr_o = (i + 1) % GSPCA_MAX_FRAMES;
if (gspca_dev->sd_desc->dq_callback) {
mutex_lock(&gspca_dev->usb_lock);
if (gspca_dev->sd_desc->dq_callback) {
mutex_lock(&gspca_dev->usb_lock);
gspca_dev->sd_desc->dq_callback(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock);
}
gspca_dev->sd_desc->dq_callback(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock);
}
+ return gspca_dev->fr_queue[i];
}
/* put the buffer in the 'queued' queue */
}
/* put the buffer in the 'queued' queue */
+ i = atomic_read(&gspca_dev->fr_q);
gspca_dev->fr_queue[i] = index;
gspca_dev->fr_queue[i] = index;
- if (gspca_dev->fr_i == i)
- gspca_dev->image = frame->data;
- gspca_dev->fr_q = (i + 1) % gspca_dev->nframes;
- PDEBUG(D_FRAM, "qbuf q:%d i:%d o:%d",
- gspca_dev->fr_q,
- gspca_dev->fr_i,
- gspca_dev->fr_o);
+ atomic_set(&gspca_dev->fr_q, (i + 1) % GSPCA_MAX_FRAMES);
v4l2_buf->flags |= V4L2_BUF_FLAG_QUEUED;
v4l2_buf->flags &= ~V4L2_BUF_FLAG_DONE;
v4l2_buf->flags |= V4L2_BUF_FLAG_QUEUED;
v4l2_buf->flags &= ~V4L2_BUF_FLAG_DONE;
static unsigned int dev_poll(struct file *file, poll_table *wait)
{
struct gspca_dev *gspca_dev = file->private_data;
static unsigned int dev_poll(struct file *file, poll_table *wait)
{
struct gspca_dev *gspca_dev = file->private_data;
if (mutex_lock_interruptible(&gspca_dev->queue_lock) != 0)
return POLLERR;
if (mutex_lock_interruptible(&gspca_dev->queue_lock) != 0)
return POLLERR;
- /* check the next incoming buffer */
- i = gspca_dev->fr_o;
- i = gspca_dev->fr_queue[i];
- if (gspca_dev->frame[i].v4l2_buf.flags & V4L2_BUF_FLAG_DONE)
- ret = POLLIN | POLLRDNORM; /* something to read */
+ /* check if an image has been received */
+ if (gspca_dev->fr_o != atomic_read(&gspca_dev->fr_i))
+ ret = POLLIN | POLLRDNORM; /* yes */
else
ret = 0;
mutex_unlock(&gspca_dev->queue_lock);
else
ret = 0;
mutex_unlock(&gspca_dev->queue_lock);
u8 *image; /* image beeing filled */
__u32 frsz; /* frame size */
u32 image_len; /* current length of image */
u8 *image; /* image beeing filled */
__u32 frsz; /* frame size */
u32 image_len; /* current length of image */
- char nframes; /* number of frames */
- char fr_i; /* frame being filled */
- char fr_q; /* next frame to queue */
- char fr_o; /* next frame to dequeue */
+ atomic_t fr_q; /* next frame to queue */
+ atomic_t fr_i; /* frame being filled */
signed char fr_queue[GSPCA_MAX_FRAMES]; /* frame queue */
signed char fr_queue[GSPCA_MAX_FRAMES]; /* frame queue */
+ char nframes; /* number of frames */
+ u8 fr_o; /* next frame to dequeue */
__u8 last_packet_type;
__s8 empty_packet; /* if (-1) don't check empty packets */
__u8 streaming;
__u8 last_packet_type;
__s8 empty_packet; /* if (-1) don't check empty packets */
__u8 streaming;
} else {
int cur_frame_len;
} else {
int cur_frame_len;
- if (gspca_dev->image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
-
cur_frame_len = gspca_dev->image_len;
/* Remove urb header */
data += 4;
cur_frame_len = gspca_dev->image_len;
/* Remove urb header */
data += 4;
/* If this packet is marked as EOF, end the frame */
} else if (data[1] & UVC_STREAM_EOF) {
sd->last_pts = 0;
/* If this packet is marked as EOF, end the frame */
} else if (data[1] & UVC_STREAM_EOF) {
sd->last_pts = 0;
- if (gspca_dev->image == NULL)
- goto discard;
if (gspca_dev->image_len + len - 12 !=
gspca_dev->width * gspca_dev->height * 2) {
PDEBUG(D_PACK, "wrong sized frame");
if (gspca_dev->image_len + len - 12 !=
gspca_dev->width * gspca_dev->height * 2) {
PDEBUG(D_PACK, "wrong sized frame");
if (sof) {
int n, lum_offset, footer_length;
if (sof) {
int n, lum_offset, footer_length;
- image = gspca_dev->image;
- if (image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
-
/* 6 bytes after the FF D9 EOF marker a number of lumination
bytes are send corresponding to different parts of the
image, the 14th and 15th byte after the EOF seem to
/* 6 bytes after the FF D9 EOF marker a number of lumination
bytes are send corresponding to different parts of the
image, the 14th and 15th byte after the EOF seem to
} else {
gspca_frame_add(gspca_dev, INTER_PACKET, data, n);
}
} else {
gspca_frame_add(gspca_dev, INTER_PACKET, data, n);
}
- if (gspca_dev->last_packet_type != DISCARD_PACKET
+
+ image = gspca_dev->image;
+ if (image != NULL
&& image[gspca_dev->image_len - 2] == 0xff
&& image[gspca_dev->image_len - 1] == 0xd9)
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
&& image[gspca_dev->image_len - 2] == 0xff
&& image[gspca_dev->image_len - 1] == 0xd9)
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
if (sof) {
int n, lum_offset, footer_length;
if (sof) {
int n, lum_offset, footer_length;
- image = gspca_dev->image;
- if (image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
-
/* 6 bytes after the FF D9 EOF marker a number of lumination
bytes are send corresponding to different parts of the
image, the 14th and 15th byte after the EOF seem to
/* 6 bytes after the FF D9 EOF marker a number of lumination
bytes are send corresponding to different parts of the
image, the 14th and 15th byte after the EOF seem to
} else {
gspca_frame_add(gspca_dev, INTER_PACKET, data, n);
}
} else {
gspca_frame_add(gspca_dev, INTER_PACKET, data, n);
}
- if (gspca_dev->last_packet_type != DISCARD_PACKET
+ image = gspca_dev->image;
+ if (image != NULL
&& image[gspca_dev->image_len - 2] == 0xff
&& image[gspca_dev->image_len - 1] == 0xd9)
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
&& image[gspca_dev->image_len - 2] == 0xff
&& image[gspca_dev->image_len - 1] == 0xd9)
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
int used;
int size = cam->cam_mode[gspca_dev->curr_mode].sizeimage;
int used;
int size = cam->cam_mode[gspca_dev->curr_mode].sizeimage;
- if (gspca_dev->image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
used = gspca_dev->image_len;
if (used + len > size)
len = size - used;
used = gspca_dev->image_len;
if (used + len > size)
len = size - used;
if (sd->bridge == BRIDGE_VC0321) {
int size, l;
if (sd->bridge == BRIDGE_VC0321) {
int size, l;
- if (gspca_dev->image == NULL) {
- gspca_dev->last_packet_type = DISCARD_PACKET;
- return;
- }
l = gspca_dev->image_len;
size = gspca_dev->frsz;
if (len > size - l)
l = gspca_dev->image_len;
size = gspca_dev->frsz;
if (len > size - l)