BUG: double mutex_unlock in drivers/media/video/tlg2300/pd-video.c

PATCH Alexander Strakh 3 participants 3 messages Dec 13, 2010 – Dec 22, 2010 Media (V4L/DVB)

Share & tools

Alexander Strakh

author

2010-12-13 15:59 UTC #0 lore raw

KERNEL_VERSION: 2.6.36
SUBJECT: double mutex_lock in drivers/media/video/tlg2300/pd-video.c
in function vidioc_s_fmt
SUBSCRIBE:
First mutex_unlock in function pd_vidioc_s_fmt in line 767:

764 ret |= send_set_req(pd, VIDEO_ROSOLU_SEL,
765 vid_resol, &cmd_status);

 766        if (ret || cmd_status) {
 767                mutex_unlock(&pd->lock);
 768                return -EBUSY;

769 }

Second mutex_unlock in function vidioc_s_fmt in line 806:

805 pd_vidioc_s_fmt(pd, &f->fmt.pix);
806 mutex_unlock(&pd->lock);

Found by Linux Device Drivers Verification Project

Сhecks the return code of pd_vidioc_s_fm before mutex_unlocking.

Signed-off-by Alexander Strakh <[email protected]>

---

(quoted diff) +0 -0

b/drivers/media/video/tlg2300/pd-video.c
index a1ffe18..fe6bd2b 100644

drivers/media/video/tlg2300/pd-video.c +0 -0

@@ -802,8 +802,8 @@ static int vidioc_s_fmt(struct file *file, void *fh,

struct v4l2_format *f)
return -EINVAL;
}

- pd_vidioc_s_fmt(pd, &f->fmt.pix);
- mutex_unlock(&pd->lock);
+ if(!pd_vidioc_s_fmt(pd, &f->fmt.pix))
+ mutex_unlock(&pd->lock);
return 0;
}

Huang Shijie

2010-12-14 01:34 UTC #1 lore raw

Hi Strakh:

Thanks for your patch.

But I prefer to remove the mutex_unlock() in the pd_vidioc_s_fmt(), since the pd_vidioc_s_fmt() is also called in restore_v4l2_context().

would you please change the patch?
I will ack it.

Best Regards
Huang Shijie

2010/12/13 Alexander Strakh <[email protected]>:

Quoted 44 lines

KERNEL_VERSION: 2.6.36
SUBJECT: double mutex_lock in drivers/media/video/tlg2300/pd-video.c in function vidioc_s_fmt
SUBSCRIBE:
First mutex_unlock in function pd_vidioc_s_fmt in line 767:
764 ret |= send_set_req(pd, VIDEO_ROSOLU_SEL, 765 vid_resol, &cmd_status); 766 if (ret || cmd_status) {
767 mutex_unlock(&pd->lock);
768 return -EBUSY;
769 }
Second mutex_unlock in function vidioc_s_fmt in line 806:
805 pd_vidioc_s_fmt(pd, &f->fmt.pix);
806 mutex_unlock(&pd->lock);
Found by Linux Device Drivers Verification Project
Сhecks the return code of pd_vidioc_s_fm before mutex_unlocking.
Signed-off-by Alexander Strakh <[email protected]>
---
drivers/media/video/tlg2300/pd-video.c +2 -2
index a1ffe18..fe6bd2b 100644
@@ -802,8 +802,8 @@ static int vidioc_s_fmt(struct file *file, void *fh,
 struct v4l2_format *f)
                return -EINVAL;
        }
 
-       pd_vidioc_s_fmt(pd, &f->fmt.pix);
-       mutex_unlock(&pd->lock);
+       if(!pd_vidioc_s_fmt(pd, &f->fmt.pix))
+               mutex_unlock(&pd->lock);
        return 0;
  }

Mauro Carvalho Chehab

2010-12-22 16:32 UTC #2 lore raw

In reply to Alexander Strakh

Em 13-12-2010 13:59, iceberg escreveu:

Quoted 37 lines

KERNEL_VERSION: 2.6.36
SUBJECT: double mutex_lock in drivers/media/video/tlg2300/pd-video.c
in function vidioc_s_fmt
SUBSCRIBE:
First mutex_unlock in function pd_vidioc_s_fmt in line 767:
764 ret |= send_set_req(pd, VIDEO_ROSOLU_SEL,
765 vid_resol, &cmd_status);
 766        if (ret || cmd_status) {
 767                mutex_unlock(&pd->lock);
 768                return -EBUSY;
769 }
Second mutex_unlock in function vidioc_s_fmt in line 806:
805 pd_vidioc_s_fmt(pd, &f->fmt.pix);
806 mutex_unlock(&pd->lock);
Found by Linux Device Drivers Verification Project
?hecks the return code of pd_vidioc_s_fm before mutex_unlocking.
Signed-off-by Alexander Strakh <[email protected]>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
drivers/media/video/tlg2300/pd-video.c +0 -0
index a1ffe18..fe6bd2b 100644

drivers/media/video/tlg2300/pd-video.c static int vidioc_s_fmt(struct file *file, void *fh, · line 809

Quoted 10 lines

drivers/media/video/tlg2300/pd-video.c +2 -2

@@ -802,8 +802,8 @@ static int vidioc_s_fmt(struct file *file, void *fh, 
 struct v4l2_format *f)
 		return -EINVAL;
 	}
 
-	pd_vidioc_s_fmt(pd, &f->fmt.pix);
-	mutex_unlock(&pd->lock);
+	if (!pd_vidioc_s_fmt(pd, &f->fmt.pix)) 
+		mutex_unlock(&pd->lock);
 	return 0;
 }

Thanks for the patch, but the better is to keep the calls to mutex_lock/mutex_unlock at the same function. So, instead of adding an "if" at vidioc_s_fmt, please remove the mutex_unlock() from pd_vidioc_s_fmt().

Cheers,
Mauro