If the last if condition passes the cdrom_mutex mutex will not be unlocked.
This patch fixes this and also removes the unnecessary doit label that was only invoked by the done label.
Thanks.
Signed-off-by: Davidlohr Bueso <[email protected]>
---
drivers/cdrom/cdrom.c | 51 +++++++++++++++++++++++++-----------------------
1 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index e3749d0..539b8d0 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -3429,70 +3429,73 @@ static int cdrom_sysctl_info(ctl_table *ctl, int write,
pos = sprintf(info, "CD-ROM information, " VERSION "\n");
if (cdrom_print_info("\ndrive name:\t", 0, info, &pos, CTL_NAME))
- goto done;
+ goto small;
if (cdrom_print_info("\ndrive speed:\t", 0, info, &pos, CTL_SPEED))
- goto done;
+ goto small;
if (cdrom_print_info("\ndrive # of slots:", 0, info, &pos, CTL_SLOTS))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan close tray:\t",
CDC_CLOSE_TRAY, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan open tray:\t",
CDC_OPEN_TRAY, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan lock tray:\t",
CDC_LOCK, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan change speed:",
CDC_SELECT_SPEED, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan select disk:",
CDC_SELECT_DISC, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan read multisession:",
CDC_MULTI_SESSION, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan read MCN:\t",
CDC_MCN, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nReports media changed:",
CDC_MEDIA_CHANGED, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan play audio:\t",
CDC_PLAY_AUDIO, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write CD-R:\t",
CDC_CD_R, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write CD-RW:",
CDC_CD_RW, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan read DVD:\t",
CDC_DVD, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write DVD-R:",
CDC_DVD_R, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write DVD-RAM:",
CDC_DVD_RAM, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan read MRW:\t",
CDC_MRW, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write MRW:\t",
CDC_MRW_W, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (cdrom_print_info("\nCan write RAM:\t",
CDC_RAM, info, &pos, CTL_CAPABILITY))
- goto done;
+ goto small;
if (!scnprintf(info + pos, max_size - pos, "\n\n"))
- goto done;
-doit:
+ goto small;
+
+ goto ret;
+
+ret:
mutex_unlock(&cdrom_mutex);
return proc_dostring(ctl, write, buffer, lenp, ppos);
-done:
+small:
printk(KERN_INFO "cdrom: info buffer too small\n");
- goto doit;
+ goto ret;
}
/* Unfortunately, per device settings are not implemented through
--
1.7.0.4
On Wed, 09 Jun 2010 12:56:50 -0400
Davidlohr Bueso <[email protected]> wrote:
> If the last if condition passes the cdrom_mutex mutex will not be unlocked.
> This patch fixes this and also removes the unnecessary doit label that was only invoked by the done label.
>
Confused. There is no way in which cdrom_sysctl_info() can forget to
unlock cdrom_mutex.
On Mon, 2010-06-14 at 12:34 -0700, Andrew Morton wrote:
> On Wed, 09 Jun 2010 12:56:50 -0400
> Davidlohr Bueso <[email protected]> wrote:
>
> > If the last if condition passes the cdrom_mutex mutex will not be unlocked.
> > This patch fixes this and also removes the unnecessary doit label that was only invoked by the done label.
> >
>
> Confused. There is no way in which cdrom_sysctl_info() can forget to
> unlock cdrom_mutex.
>
Ok, I had misread the code then, one of the reasons I don't like goto
labels, but that's a different story.
Regarding cdrom_sysctl_info(), I have noticed in several netbooks
(haven't
looked in other machines with no cdrom drive) that the
/proc/sys/dev/cdrom/info file shows no values for the listed entries,
ie:
CD-ROM information, Id: cdrom.c 3.20 2003/12/17
drive name:
drive speed:
drive # of slots:
Can close tray:
Can open tray:
...
Now, is this expected behavior? If not, should it be changed to
something
more descriptive? Perhaps it would affect user space applications that
read
the file?
Thanks,
Davidlohr
On Tue, 15 Jun 2010 14:40:54 -0400
Davidlohr Bueso <[email protected]> wrote:
> Regarding cdrom_sysctl_info(), I have noticed in several netbooks
> (haven't
> looked in other machines with no cdrom drive) that the
> /proc/sys/dev/cdrom/info file shows no values for the listed entries,
> ie:
>
> CD-ROM information, Id: cdrom.c 3.20 2003/12/17
>
> drive name:
> drive speed:
> drive # of slots:
> Can close tray:
> Can open tray:
> ...
>
> Now, is this expected behavior? If not, should it be changed to
> something
> more descriptive? Perhaps it would affect user space applications that
> read
> the file?
It works for me.
Does your kernel build have CONFIG_SYSCTL=n? If so, then all that code
is disabled. That seems a bit bogus to me and perhaps we should enable
at least some of that code by CONFIG_PROCFS instead.