* [patch] cdrom: Handle TOC
@ 2008-08-06 21:03 Alexander Inyukhin
2008-08-06 23:11 ` Tejun Heo
2008-08-07 4:18 ` Borislav Petkov
0 siblings, 2 replies; 5+ messages in thread
From: Alexander Inyukhin @ 2008-08-06 21:03 UTC (permalink / raw)
To: linux-ide
Cc: Tejun Heo, Jens Axboe, Borislav Petkov, Andrew Morton,
Bartlomiej Zolnierkiewicz
[-- Attachment #1: Type: text/plain, Size: 585 bytes --]
Hi
This patch should fix TOC handling for cdroms that can not play audio.
It extends commit af744e3294d09d706c4eae26cffaaa68a8d40337
with a safety check and non-audio ioctls support.
Since CDC_PLAY_AUDIO flag was used not only to check ability
to play audio but also to ensure that audio_ioctl was not NULL,
all TOC-related operations had to use it.
As far as I understand, now audio_ioctl is never NULL,
so a sanity check during device registration should be sufficient.
The patch is against 2.6.27-rc2. It was tested on Optiarc AD7203A
device, that has no ability to play audio.
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1566 bytes --]
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index d9d1b65..efc3aa0 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -394,6 +394,8 @@ int register_cdrom(struct cdrom_device_info *cdi)
if (cdo->open == NULL || cdo->release == NULL)
return -EINVAL;
+ if (cdo->audio_ioctl == NULL)
+ return -EINVAL;
if (!banner_printed) {
printk(KERN_INFO "Uniform CD-ROM driver " REVISION "\n");
banner_printed = 1;
@@ -408,7 +410,6 @@ int register_cdrom(struct cdrom_device_info *cdi)
ENSURE(get_last_session, CDC_MULTI_SESSION);
ENSURE(get_mcn, CDC_MCN);
ENSURE(reset, CDC_RESET);
- ENSURE(audio_ioctl, CDC_PLAY_AUDIO);
ENSURE(generic_packet, CDC_GENERIC_PACKET);
cdi->mc_flags = 0;
cdo->n_minors = 0;
@@ -2506,8 +2507,6 @@ static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
/* cdinfo(CD_DO_IOCTL,"entering CDROMSUBCHNL\n");*/
- if (!CDROM_CAN(CDC_PLAY_AUDIO))
- return -ENOSYS;
if (copy_from_user(&q, argp, sizeof(q)))
return -EFAULT;
@@ -2538,8 +2537,6 @@ static int cdrom_ioctl_read_tochdr(struct cdrom_device_info *cdi,
/* cdinfo(CD_DO_IOCTL, "entering CDROMREADTOCHDR\n"); */
- if (!CDROM_CAN(CDC_PLAY_AUDIO))
- return -ENOSYS;
if (copy_from_user(&header, argp, sizeof(header)))
return -EFAULT;
@@ -2562,8 +2559,6 @@ static int cdrom_ioctl_read_tocentry(struct cdrom_device_info *cdi,
/* cdinfo(CD_DO_IOCTL, "entering CDROMREADTOCENTRY\n"); */
- if (!CDROM_CAN(CDC_PLAY_AUDIO))
- return -ENOSYS;
if (copy_from_user(&entry, argp, sizeof(entry)))
return -EFAULT;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [patch] cdrom: Handle TOC
2008-08-06 21:03 [patch] cdrom: Handle TOC Alexander Inyukhin
@ 2008-08-06 23:11 ` Tejun Heo
2008-08-07 4:23 ` Borislav Petkov
2008-08-07 4:18 ` Borislav Petkov
1 sibling, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2008-08-06 23:11 UTC (permalink / raw)
To: Alexander Inyukhin
Cc: linux-ide, Jens Axboe, Borislav Petkov, Andrew Morton,
Bartlomiej Zolnierkiewicz
Alexander Inyukhin wrote:
> index d9d1b65..efc3aa0 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -394,6 +394,8 @@ int register_cdrom(struct cdrom_device_info *cdi)
>
> if (cdo->open == NULL || cdo->release == NULL)
> return -EINVAL;
> + if (cdo->audio_ioctl == NULL)
> + return -EINVAL;
Maybe sticking a BUG_ON() is a good idea just in case?
--
tejun
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] cdrom: Handle TOC
2008-08-06 23:11 ` Tejun Heo
@ 2008-08-07 4:23 ` Borislav Petkov
0 siblings, 0 replies; 5+ messages in thread
From: Borislav Petkov @ 2008-08-07 4:23 UTC (permalink / raw)
To: Tejun Heo
Cc: Alexander Inyukhin, linux-ide, Jens Axboe, Andrew Morton,
Bartlomiej Zolnierkiewicz
On Thu, Aug 07, 2008 at 08:11:45AM +0900, Tejun Heo wrote:
> Alexander Inyukhin wrote:
> > index d9d1b65..efc3aa0 100644
> > --- a/drivers/cdrom/cdrom.c
> > +++ b/drivers/cdrom/cdrom.c
> > @@ -394,6 +394,8 @@ int register_cdrom(struct cdrom_device_info *cdi)
> >
> > if (cdo->open == NULL || cdo->release == NULL)
> > return -EINVAL;
> > + if (cdo->audio_ioctl == NULL)
> > + return -EINVAL;
>
> Maybe sticking a BUG_ON() is a good idea just in case?
I'd do it only temporary so that we could catch drivers which do not set
cdo->audio_ioctl and then remove it after having fixed those. Otherwise it is a
bit too much, imho.
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] cdrom: Handle TOC
2008-08-06 21:03 [patch] cdrom: Handle TOC Alexander Inyukhin
2008-08-06 23:11 ` Tejun Heo
@ 2008-08-07 4:18 ` Borislav Petkov
2008-08-07 17:11 ` Alexander Inyukhin
1 sibling, 1 reply; 5+ messages in thread
From: Borislav Petkov @ 2008-08-07 4:18 UTC (permalink / raw)
To: Alexander Inyukhin
Cc: linux-ide, Tejun Heo, Jens Axboe, Andrew Morton,
Bartlomiej Zolnierkiewicz
On Thu, Aug 07, 2008 at 01:03:58AM +0400, Alexander Inyukhin wrote:
> Hi
>
> This patch should fix TOC handling for cdroms that can not play audio.
> It extends commit af744e3294d09d706c4eae26cffaaa68a8d40337
> with a safety check and non-audio ioctls support.
>
> Since CDC_PLAY_AUDIO flag was used not only to check ability
> to play audio but also to ensure that audio_ioctl was not NULL,
> all TOC-related operations had to use it.
>
> As far as I understand, now audio_ioctl is never NULL,
> so a sanity check during device registration should be sufficient.
>
> The patch is against 2.6.27-rc2. It was tested on Optiarc AD7203A
> device, that has no ability to play audio.
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
> index d9d1b65..efc3aa0 100644
> --- a/drivers/cdrom/cdrom.c
> +++ b/drivers/cdrom/cdrom.c
> @@ -394,6 +394,8 @@ int register_cdrom(struct cdrom_device_info *cdi)
>
> if (cdo->open == NULL || cdo->release == NULL)
> return -EINVAL;
> + if (cdo->audio_ioctl == NULL)
> + return -EINVAL;
How about putting all checks above together:
if (!cdo->open || !cdo->release || !cdo->audio_ioctl)
return -EINVAL;
?
> if (!banner_printed) {
> printk(KERN_INFO "Uniform CD-ROM driver " REVISION "\n");
> banner_printed = 1;
> @@ -408,7 +410,6 @@ int register_cdrom(struct cdrom_device_info *cdi)
> ENSURE(get_last_session, CDC_MULTI_SESSION);
> ENSURE(get_mcn, CDC_MCN);
> ENSURE(reset, CDC_RESET);
> - ENSURE(audio_ioctl, CDC_PLAY_AUDIO);
> ENSURE(generic_packet, CDC_GENERIC_PACKET);
> cdi->mc_flags = 0;
> cdo->n_minors = 0;
> @@ -2506,8 +2507,6 @@ static int cdrom_ioctl_get_subchnl(struct cdrom_device_info *cdi,
>
> /* cdinfo(CD_DO_IOCTL,"entering CDROMSUBCHNL\n");*/
>
> - if (!CDROM_CAN(CDC_PLAY_AUDIO))
> - return -ENOSYS;
> if (copy_from_user(&q, argp, sizeof(q)))
> return -EFAULT;
>
> @@ -2538,8 +2537,6 @@ static int cdrom_ioctl_read_tochdr(struct cdrom_device_info *cdi,
>
> /* cdinfo(CD_DO_IOCTL, "entering CDROMREADTOCHDR\n"); */
>
> - if (!CDROM_CAN(CDC_PLAY_AUDIO))
> - return -ENOSYS;
> if (copy_from_user(&header, argp, sizeof(header)))
> return -EFAULT;
>
> @@ -2562,8 +2559,6 @@ static int cdrom_ioctl_read_tocentry(struct cdrom_device_info *cdi,
>
> /* cdinfo(CD_DO_IOCTL, "entering CDROMREADTOCENTRY\n"); */
>
> - if (!CDROM_CAN(CDC_PLAY_AUDIO))
> - return -ENOSYS;
> if (copy_from_user(&entry, argp, sizeof(entry)))
> return -EFAULT;
>
--
Regards/Gruss,
Boris.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch] cdrom: Handle TOC
2008-08-07 4:18 ` Borislav Petkov
@ 2008-08-07 17:11 ` Alexander Inyukhin
0 siblings, 0 replies; 5+ messages in thread
From: Alexander Inyukhin @ 2008-08-07 17:11 UTC (permalink / raw)
To: petkovbb
Cc: linux-ide, Tejun Heo, Jens Axboe, Andrew Morton,
Bartlomiej Zolnierkiewicz
On Thu, Aug 07, 2008 at 06:18:37AM +0200, Borislav Petkov wrote:
> On Thu, Aug 07, 2008 at 01:03:58AM +0400, Alexander Inyukhin wrote:
> > if (cdo->open == NULL || cdo->release == NULL)
> > return -EINVAL;
> > + if (cdo->audio_ioctl == NULL)
> > + return -EINVAL;
>
> How about putting all checks above together:
> if (!cdo->open || !cdo->release || !cdo->audio_ioctl)
> return -EINVAL;
>
> ?
I thought to place a warning message here and it is easier to blame separate check.
Feel free to modify these lines.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-08-07 17:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-06 21:03 [patch] cdrom: Handle TOC Alexander Inyukhin
2008-08-06 23:11 ` Tejun Heo
2008-08-07 4:23 ` Borislav Petkov
2008-08-07 4:18 ` Borislav Petkov
2008-08-07 17:11 ` Alexander Inyukhin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.