Skip to content

L
linux
Commit 587f0d5d authored by Mauro Carvalho Chehab's avatar Mauro Carvalho Chehab Committed by Mauro Carvalho Chehab
Browse files
Options

[media] bttv: fix mutex use before init (BZ#24602) 

Fix a regression where bttv driver causes oopses when loading, since it
were using some non-initialized mutexes. While it would be possible to
fix the issue, there are some other lock troubles, like to the presence of
lock code at free_btres_lock().

It is possible to fix, but the better is to just use the core-assisted
locking schema. This way, V4L2 core will serialize access to all
ioctl's/open/close/mmap/read/poll operations, avoiding to have two
processes accessing the hardware at the same time. Also, as there's just
one lock, instead of 3, there's no risk of dead locks.

The net result is a cleaner code, with just one lock.
Reported-by: default avatarDan Carpenter <error27@gmail.com>
Reported-by: Brandon Philips<brandon@ifup.org>
Reported-by: default avatarChris Clayton <chris2553@googlemail.com>
Reported-by: default avatarTorsten Kaiser <just.for.lkml@googlemail.com>
Tested-by: default avatarChris Clayton <chris2553@googlemail.com>
Tested-by: default avatarTorsten Kaiser <just.for.lkml@googlemail.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent a757ee22
Show whitespace changes
Inline Side-by-side
Showing with 3 additions and 94 deletions
drivers/media/video/bt8xx/bttv-driver.c
View file @ 587f0d5d
......@@ -855,7 +855,6 @@ int check_alloc_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bit)
xbits |= RESOURCE_VIDEO_READ | RESOURCE_VIDEO_STREAM;
/* is it free? */
mutex_lock(&btv->lock);
if (btv->resources & xbits) {
/* no, someone else uses it */
goto fail;
......@@ -885,11 +884,9 @@ int check_alloc_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bit)
/* it's free, grab it */
fh->resources |= bit;
btv->resources |= bit;
mutex_unlock(&btv->lock);
return 1;
fail:
mutex_unlock(&btv->lock);
return 0;
}
......@@ -941,7 +938,6 @@ void free_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bits)
/* trying to free ressources not allocated by us ... */
printk("bttv: BUG! (btres)\n");
}
mutex_lock(&btv->lock);
fh->resources &= ~bits;
btv->resources &= ~bits;
......@@ -952,8 +948,6 @@ void free_btres_lock(struct bttv *btv, struct bttv_fh *fh, int bits)
if (0 == (bits & VBI_RESOURCES))
disclaim_vbi_lines(btv);
mutex_unlock(&btv->lock);
}
/* ----------------------------------------------------------------------- */
......@@ -1714,28 +1708,20 @@ static int bttv_prepare_buffer(struct videobuf_queue *q,struct bttv *btv,
/* Make sure tvnorm and vbi_end remain consistent
until we're done. */
mutex_lock(&btv->lock);
norm = btv->tvnorm;
/* In this mode capturing always starts at defrect.top
(default VDELAY), ignoring cropping parameters. */
if (btv->vbi_end > bttv_tvnorms[norm].cropcap.defrect.top) {
mutex_unlock(&btv->lock);
return -EINVAL;
}
mutex_unlock(&btv->lock);
c.rect = bttv_tvnorms[norm].cropcap.defrect;
} else {
mutex_lock(&btv->lock);
norm = btv->tvnorm;
c = btv->crop[!!fh->do_crop];
mutex_unlock(&btv->lock);
if (width < c.min_scaled_width ||
width > c.max_scaled_width ||
height < c.min_scaled_height)
......@@ -1859,7 +1845,6 @@ static int bttv_s_std(struct file *file, void *priv, v4l2_std_id *id)
unsigned int i;
int err;
mutex_lock(&btv->lock);
err = v4l2_prio_check(&btv->prio, fh->prio);
if (err)
goto err;
......@@ -1875,7 +1860,6 @@ static int bttv_s_std(struct file *file, void *priv, v4l2_std_id *id)
set_tvnorm(btv, i);
err:
mutex_unlock(&btv->lock);
return err;
}
......@@ -1899,7 +1883,6 @@ static int bttv_enum_input(struct file *file, void *priv,
struct bttv *btv = fh->btv;
int rc = 0;
mutex_lock(&btv->lock);
if (i->index >= bttv_tvcards[btv->c.type].video_inputs) {
rc = -EINVAL;
goto err;
......@@ -1929,7 +1912,6 @@ static int bttv_enum_input(struct file *file, void *priv,
i->std = BTTV_NORMS;
err:
mutex_unlock(&btv->lock);
return rc;
}
......@@ -1939,9 +1921,7 @@ static int bttv_g_input(struct file *file, void *priv, unsigned int *i)
struct bttv_fh *fh = priv;
struct bttv *btv = fh->btv;
mutex_lock(&btv->lock);
*i = btv->input;
mutex_unlock(&btv->lock);
return 0;
}
......@@ -1953,7 +1933,6 @@ static int bttv_s_input(struct file *file, void *priv, unsigned int i)
int err;
mutex_lock(&btv->lock);
err = v4l2_prio_check(&btv->prio, fh->prio);
if (unlikely(err))
goto err;
......@@ -1966,7 +1945,6 @@ static int bttv_s_input(struct file *file, void *priv, unsigned int i)
set_input(btv, i, btv->tvnorm);
err:
mutex_unlock(&btv->lock);
return 0;
}
......@@ -1980,7 +1958,6 @@ static int bttv_s_tuner(struct file *file, void *priv,
if (unlikely(0 != t->index))
return -EINVAL;
mutex_lock(&btv->lock);
if (unlikely(btv->tuner_type == TUNER_ABSENT)) {
err = -EINVAL;
goto err;
......@@ -1996,7 +1973,6 @@ static int bttv_s_tuner(struct file *file, void *priv,
btv->audio_mode_gpio(btv, t, 1);
err:
mutex_unlock(&btv->lock);
return 0;
}
......@@ -2007,10 +1983,8 @@ static int bttv_g_frequency(struct file *file, void *priv,
struct bttv_fh *fh = priv;
struct bttv *btv = fh->btv;
mutex_lock(&btv->lock);
f->type = btv->radio_user ? V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
f->frequency = btv->freq;
mutex_unlock(&btv->lock);
return 0;
}
......@@ -2025,7 +1999,6 @@ static int bttv_s_frequency(struct file *file, void *priv,
if (unlikely(f->tuner != 0))
return -EINVAL;
mutex_lock(&btv->lock);
err = v4l2_prio_check(&btv->prio, fh->prio);
if (unlikely(err))
goto err;
......@@ -2040,7 +2013,6 @@ static int bttv_s_frequency(struct file *file, void *priv,
if (btv->has_matchbox && btv->radio_user)
tea5757_set_freq(btv, btv->freq);
err:
mutex_unlock(&btv->lock);
return 0;
}
......@@ -2173,7 +2145,6 @@ limit_scaled_size_lock (struct bttv_fh * fh,
/* Make sure tvnorm, vbi_end and the current cropping parameters
remain consistent until we're done. */
mutex_lock(&btv->lock);
b = &bttv_tvnorms[btv->tvnorm].cropcap.bounds;
......@@ -2251,7 +2222,6 @@ limit_scaled_size_lock (struct bttv_fh * fh,
rc = 0; /* success */
fail:
mutex_unlock(&btv->lock);
return rc;
}
......@@ -2283,9 +2253,7 @@ verify_window_lock (struct bttv_fh * fh,
if (V4L2_FIELD_ANY == field) {
__s32 height2;
mutex_lock(&fh->btv->lock);
height2 = fh->btv->crop[!!fh->do_crop].rect.height >> 1;
mutex_unlock(&fh->btv->lock);
field = (win->w.height > height2)
? V4L2_FIELD_INTERLACED
: V4L2_FIELD_TOP;
......@@ -2361,7 +2329,6 @@ static int setup_window_lock(struct bttv_fh *fh, struct bttv *btv,
}
}
mutex_lock(&fh->cap.vb_lock);
/* clip against screen */
if (NULL != btv->fbuf.base)
n = btcx_screen_clips(btv->fbuf.fmt.width, btv->fbuf.fmt.height,
......@@ -2413,7 +2380,6 @@ static int setup_window_lock(struct bttv_fh *fh, struct bttv *btv,
bttv_overlay_risc(btv, &fh->ov, fh->ovfmt, new);
retval = bttv_switch_overlay(btv,fh,new);
}
mutex_unlock(&fh->cap.vb_lock);
return retval;
}
......@@ -2527,9 +2493,7 @@ static int bttv_try_fmt_vid_cap(struct file *file, void *priv,
if (V4L2_FIELD_ANY == field) {
__s32 height2;
mutex_lock(&btv->lock);
height2 = btv->crop[!!fh->do_crop].rect.height >> 1;
mutex_unlock(&btv->lock);
field = (f->fmt.pix.height > height2)
? V4L2_FIELD_INTERLACED
: V4L2_FIELD_BOTTOM;
......@@ -2615,7 +2579,6 @@ static int bttv_s_fmt_vid_cap(struct file *file, void *priv,
fmt = format_by_fourcc(f->fmt.pix.pixelformat);
/* update our state informations */
mutex_lock(&fh->cap.vb_lock);
fh->fmt = fmt;
fh->cap.field = f->fmt.pix.field;
fh->cap.last = V4L2_FIELD_NONE;
......@@ -2624,7 +2587,6 @@ static int bttv_s_fmt_vid_cap(struct file *file, void *priv,
btv->init.fmt = fmt;
btv->init.width = f->fmt.pix.width;
btv->init.height = f->fmt.pix.height;
mutex_unlock(&fh->cap.vb_lock);
return 0;
}
......@@ -2650,11 +2612,9 @@ static int vidiocgmbuf(struct file *file, void *priv, struct video_mbuf *mbuf)
unsigned int i;
struct bttv_fh *fh = priv;
mutex_lock(&fh->cap.vb_lock);
retval = __videobuf_mmap_setup(&fh->cap, gbuffers, gbufsize,
V4L2_MEMORY_MMAP);
if (retval < 0) {
mutex_unlock(&fh->cap.vb_lock);
return retval;
}
......@@ -2666,7 +2626,6 @@ static int vidiocgmbuf(struct file *file, void *priv, struct video_mbuf *mbuf)
for (i = 0; i < gbuffers; i++)
mbuf->offsets[i] = i * gbufsize;
mutex_unlock(&fh->cap.vb_lock);
return 0;
}
#endif
......@@ -2776,10 +2735,8 @@ static int bttv_overlay(struct file *file, void *f, unsigned int on)
int retval = 0;
if (on) {
mutex_lock(&fh->cap.vb_lock);
/* verify args */
if (unlikely(!btv->fbuf.base)) {
mutex_unlock(&fh->cap.vb_lock);
return -EINVAL;
}
if (unlikely(!fh->ov.setup_ok)) {
......@@ -2788,13 +2745,11 @@ static int bttv_overlay(struct file *file, void *f, unsigned int on)
}
if (retval)
return retval;
mutex_unlock(&fh->cap.vb_lock);
}
if (!check_alloc_btres_lock(btv, fh, RESOURCE_OVERLAY))
return -EBUSY;
mutex_lock(&fh->cap.vb_lock);
if (on) {
fh->ov.tvnorm = btv->tvnorm;
new = videobuf_sg_alloc(sizeof(*new));
......@@ -2806,7 +2761,6 @@ static int bttv_overlay(struct file *file, void *f, unsigned int on)
/* switch over */
retval = bttv_switch_overlay(btv, fh, new);
mutex_unlock(&fh->cap.vb_lock);
return retval;
}
......@@ -2845,7 +2799,6 @@ static int bttv_s_fbuf(struct file *file, void *f,
}
/* ok, accept it */
mutex_lock(&fh->cap.vb_lock);
btv->fbuf.base = fb->base;
btv->fbuf.fmt.width = fb->fmt.width;
btv->fbuf.fmt.height = fb->fmt.height;
......@@ -2877,7 +2830,6 @@ static int bttv_s_fbuf(struct file *file, void *f,
retval = bttv_switch_overlay(btv, fh, new);
}
}
mutex_unlock(&fh->cap.vb_lock);
return retval;
}
......@@ -2956,7 +2908,6 @@ static int bttv_queryctrl(struct file *file, void *priv,
c->id >= V4L2_CID_PRIVATE_LASTP1))
return -EINVAL;
mutex_lock(&btv->lock);
if (!btv->volume_gpio && (c->id == V4L2_CID_AUDIO_VOLUME))
*c = no_ctl;
else {
......@@ -2964,7 +2915,6 @@ static int bttv_queryctrl(struct file *file, void *priv,
*c = (NULL != ctrl) ? *ctrl : no_ctl;
}
mutex_unlock(&btv->lock);
return 0;
}
......@@ -2975,10 +2925,8 @@ static int bttv_g_parm(struct file *file, void *f,
struct bttv_fh *fh = f;
struct bttv *btv = fh->btv;
mutex_lock(&btv->lock);
v4l2_video_std_frame_period(bttv_tvnorms[btv->tvnorm].v4l2_id,
&parm->parm.capture.timeperframe);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -2994,7 +2942,6 @@ static int bttv_g_tuner(struct file *file, void *priv,
if (0 != t->index)
return -EINVAL;
mutex_lock(&btv->lock);
t->rxsubchans = V4L2_TUNER_SUB_MONO;
bttv_call_all(btv, tuner, g_tuner, t);
strcpy(t->name, "Television");
......@@ -3006,7 +2953,6 @@ static int bttv_g_tuner(struct file *file, void *priv,
if (btv->audio_mode_gpio)
btv->audio_mode_gpio(btv, t, 0);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3015,9 +2961,7 @@ static int bttv_g_priority(struct file *file, void *f, enum v4l2_priority *p)
struct bttv_fh *fh = f;
struct bttv *btv = fh->btv;
mutex_lock(&btv->lock);
*p = v4l2_prio_max(&btv->prio);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3029,9 +2973,7 @@ static int bttv_s_priority(struct file *file, void *f,
struct bttv *btv = fh->btv;
int rc;
mutex_lock(&btv->lock);
rc = v4l2_prio_change(&btv->prio, &fh->prio, prio);
mutex_unlock(&btv->lock);
return rc;
}
......@@ -3046,9 +2988,7 @@ static int bttv_cropcap(struct file *file, void *priv,
cap->type != V4L2_BUF_TYPE_VIDEO_OVERLAY)
return -EINVAL;
mutex_lock(&btv->lock);
*cap = bttv_tvnorms[btv->tvnorm].cropcap;
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3066,9 +3006,7 @@ static int bttv_g_crop(struct file *file, void *f, struct v4l2_crop *crop)
inconsistent with fh->width or fh->height and apps
do not expect a change here. */
mutex_lock(&btv->lock);
crop->c = btv->crop[!!fh->do_crop].rect;
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3092,17 +3030,14 @@ static int bttv_s_crop(struct file *file, void *f, struct v4l2_crop *crop)
/* Make sure tvnorm, vbi_end and the current cropping
parameters remain consistent until we're done. Note
read() may change vbi_end in check_alloc_btres_lock(). */
mutex_lock(&btv->lock);
retval = v4l2_prio_check(&btv->prio, fh->prio);
if (0 != retval) {
mutex_unlock(&btv->lock);
return retval;
}
retval = -EBUSY;
if (locked_btres(fh->btv, VIDEO_RESOURCES)) {
mutex_unlock(&btv->lock);
return retval;
}
......@@ -3114,7 +3049,6 @@ static int bttv_s_crop(struct file *file, void *f, struct v4l2_crop *crop)
b_top = max(b->top, btv->vbi_end);
if (b_top + 32 >= b_bottom) {
mutex_unlock(&btv->lock);
return retval;
}
......@@ -3137,12 +3071,8 @@ static int bttv_s_crop(struct file *file, void *f, struct v4l2_crop *crop)
btv->crop[1] = c;
mutex_unlock(&btv->lock);
fh->do_crop = 1;
mutex_lock(&fh->cap.vb_lock);
if (fh->width < c.min_scaled_width) {
fh->width = c.min_scaled_width;
btv->init.width = c.min_scaled_width;
......@@ -3159,8 +3089,6 @@ static int bttv_s_crop(struct file *file, void *f, struct v4l2_crop *crop)
btv->init.height = c.max_scaled_height;
}
mutex_unlock(&fh->cap.vb_lock);
return 0;
}
......@@ -3228,7 +3156,6 @@ static unsigned int bttv_poll(struct file *file, poll_table *wait)
return videobuf_poll_stream(file, &fh->vbi, wait);
}
mutex_lock(&fh->cap.vb_lock);
if (check_btres(fh,RESOURCE_VIDEO_STREAM)) {
/* streaming capture */
if (list_empty(&fh->cap.stream))
......@@ -3263,7 +3190,6 @@ static unsigned int bttv_poll(struct file *file, poll_table *wait)
else
rc = 0;
err:
mutex_unlock(&fh->cap.vb_lock);
return rc;
}
......@@ -3303,14 +3229,11 @@ static int bttv_open(struct file *file)
* Let's first copy btv->init at fh, holding cap.vb_lock, and then work
* with the rest of init, holding btv->lock.
*/
mutex_lock(&fh->cap.vb_lock);
*fh = btv->init;
mutex_unlock(&fh->cap.vb_lock);
fh->type = type;
fh->ov.setup_ok = 0;
mutex_lock(&btv->lock);
v4l2_prio_open(&btv->prio, &fh->prio);
videobuf_queue_sg_init(&fh->cap, &bttv_video_qops,
......@@ -3318,13 +3241,13 @@ static int bttv_open(struct file *file)
V4L2_BUF_TYPE_VIDEO_CAPTURE,
V4L2_FIELD_INTERLACED,
sizeof(struct bttv_buffer),
fh, NULL);
fh, &btv->lock);
videobuf_queue_sg_init(&fh->vbi, &bttv_vbi_qops,
&btv->c.pci->dev, &btv->s_lock,
V4L2_BUF_TYPE_VBI_CAPTURE,
V4L2_FIELD_SEQ_TB,
sizeof(struct bttv_buffer),
fh, NULL);
fh, &btv->lock);
set_tvnorm(btv,btv->tvnorm);
set_input(btv, btv->input, btv->tvnorm);
......@@ -3347,7 +3270,6 @@ static int bttv_open(struct file *file)
bttv_vbi_fmt_reset(&fh->vbi_fmt, btv->tvnorm);
bttv_field_count(btv);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3356,7 +3278,6 @@ static int bttv_release(struct file *file)
struct bttv_fh *fh = file->private_data;
struct bttv *btv = fh->btv;
mutex_lock(&btv->lock);
/* turn off overlay */
if (check_btres(fh, RESOURCE_OVERLAY))
bttv_switch_overlay(btv,fh,NULL);
......@@ -3386,10 +3307,8 @@ static int bttv_release(struct file *file)
* videobuf uses cap.vb_lock - we should avoid holding btv->lock,
* otherwise we may have dead lock conditions
*/
mutex_unlock(&btv->lock);
videobuf_mmap_free(&fh->cap);
videobuf_mmap_free(&fh->vbi);
mutex_lock(&btv->lock);
v4l2_prio_close(&btv->prio, fh->prio);
file->private_data = NULL;
kfree(fh);
......@@ -3399,7 +3318,6 @@ static int bttv_release(struct file *file)
if (!btv->users)
audio_mute(btv, 1);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3503,11 +3421,8 @@ static int radio_open(struct file *file)
if (unlikely(!fh))
return -ENOMEM;
file->private_data = fh;
mutex_lock(&fh->cap.vb_lock);
*fh = btv->init;
mutex_unlock(&fh->cap.vb_lock);
mutex_lock(&btv->lock);
v4l2_prio_open(&btv->prio, &fh->prio);
btv->radio_user++;
......@@ -3515,7 +3430,6 @@ static int radio_open(struct file *file)
bttv_call_all(btv, tuner, s_radio);
audio_input(btv,TVAUDIO_INPUT_RADIO);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3525,7 +3439,6 @@ static int radio_release(struct file *file)
struct bttv *btv = fh->btv;
struct rds_command cmd;
mutex_lock(&btv->lock);
v4l2_prio_close(&btv->prio, fh->prio);
file->private_data = NULL;
kfree(fh);
......@@ -3533,7 +3446,6 @@ static int radio_release(struct file *file)
btv->radio_user--;
bttv_call_all(btv, core, ioctl, RDS_CMD_CLOSE, &cmd);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3562,7 +3474,6 @@ static int radio_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
return -EINVAL;
if (0 != t->index)
return -EINVAL;
mutex_lock(&btv->lock);
strcpy(t->name, "Radio");
t->type = V4L2_TUNER_RADIO;
......@@ -3571,8 +3482,6 @@ static int radio_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
if (btv->audio_mode_gpio)
btv->audio_mode_gpio(btv, t, 0);
mutex_unlock(&btv->lock);
return 0;
}
......@@ -3693,7 +3602,7 @@ static const struct v4l2_file_operations radio_fops =
.open = radio_open,
.read = radio_read,
.release = radio_release,
.ioctl = video_ioctl2,
.unlocked_ioctl = video_ioctl2,
.poll = radio_poll,
};
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7