Re: kernel 2.4.27-10: isofs driver ignore some parameters with mount

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

Hi folks,

On Fri, Aug 12, 2005 at 05:29:36PM +0900, Horms wrote:
> On Fri, Aug 12, 2005 at 10:44:17AM +0300, Alexander Pytlev wrote:
> > Hello Debian,
> > 
> > Kernel 2.4.27-10
> > With mount isofs filesystem, any mount parameters after
> > iocharset=,map=,session= are ignored.
> > 
> > Sample:
> > 
> > mount -t isofs -o uid=100,iocharset=koi8-r,gid=100 /dev/cdrom /media/cdrom
> > 
> > gid=100 - was ignored
> > 
> > I look in source and find that problem. I make two patch, simply and full
> > (what addeded some functionality - ignore wrong mount parameters)
> 
> Thanks,
> 
> I will try and get the simple version of this patch into the next
> Sarge update.
> 
> I have also CCed Marcelo and the LKML for their consideration,
> as this problem still seems to be present in the lastest 2.4 tree.
> 
> -- 
> Horms
> 
> simply patch:
> ===================================================================================
> --- kernel-source-2.4.27/fs/isofs/inode.c       2005-05-19 13:29:39.000000000 +0300
> +++ kernel-source/fs/isofs/inode.c      2005-08-11 11:55:12.000000000 +0300
> @@ -340,13 +340,13 @@
>                         else if (!strcmp(value,"acorn")) popt->map = 'a';
>                         else return 0;
>                 }
> -               if (!strcmp(this_char,"session") && value) {
> +               else if (!strcmp(this_char,"session") && value) {
>                         char * vpnt = value;
>                         unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>                         if(ivalue < 0 || ivalue >99) return 0;
>                         popt->session=ivalue+1;
>                 }
> -               if (!strcmp(this_char,"sbsector") && value) {
> +               else if (!strcmp(this_char,"sbsector") && value) {
>                         char * vpnt = value;
>                         unsigned int ivalue = simple_strtoul(vpnt, &vpnt, 0);
>                         if(ivalue < 0 || ivalue >660*512) return 0;
> ===================================================================================

Neither "sbsector" or "session" parameters are part of the options string used 
in Alexander's example, so how come this patch can make any difference? 

Usage of "sbsector" or "session" parameters could explain the above patch
making a difference because the buggy, always true "(unsigned long) ivalue < 0"
comparison invokes "return 0", but that is not the case.

The code after the "popt->iocharset = value;" does not make any sense.

It seems that the "*value = 0" assignment can screw up the rest of the
string, isnt that the real issue?

#ifdef CONFIG_JOLIET
                if (!strcmp(this_char,"iocharset") && value) {
                        popt->iocharset = value;
                        while (*value && *value != ',')
                                value++;
                        if (value == popt->iocharset)
                                return 0;
                        *value = 0;
                } else
#endif