[PATCH v2 0/3] handle large user and group ID for isofs and udf

[PATCH v2 0/3] handle large user and group ID for isofs and udf

[bingjingc]

From: BingJing Chang <bingjingc@synology.com>

The uid/gid (unsigned int) of a domain user may be larger than INT_MAX.
The parse_options of isofs and udf will return 0, and mount will fail
with -EINVAL. These patches try to handle large user and group ID.

BingJing Chang (3):
  parser: add unsigned int parser
  isofs: handle large user and group ID
  udf: handle large user and group ID

 fs/isofs/inode.c       |  9 +++++----
 fs/udf/super.c         |  9 +++++----
 include/linux/parser.h |  1 +
 lib/parser.c           | 44 +++++++++++++++++++++++++++++++++-----------
 4 files changed, 44 insertions(+), 19 deletions(-)

-- 
2.7.4

Re: [PATCH v2 0/3] handle large user and group ID for isofs and udf

[Jan Kara]

On Thu 28-01-21 15:12:27, bingjingc wrote:
> From: BingJing Chang <bingjingc@synology.com>
> 
> The uid/gid (unsigned int) of a domain user may be larger than INT_MAX.
> The parse_options of isofs and udf will return 0, and mount will fail
> with -EINVAL. These patches try to handle large user and group ID.
> 
> BingJing Chang (3):
>   parser: add unsigned int parser
>   isofs: handle large user and group ID
>   udf: handle large user and group ID

Thanks for your patches! Just two notes:

1) I don't think Matthew Wilcox gave you his Reviewed-by tag (at least I
didn't see such email). Generally the rule is that the developer has to
explicitely write in his email that you can attach his Reviewed-by tag for
it to be valid.

2) Please split the cleanup of kernel doc comments in lib/parser.c in a
separate patch. It is unrelated to adding parse_uint() function so it
belongs into a separate patch (we occasionally do include small unrelated
cleanups if they are on the same line we change anyway but your changes
definitely triggered my treshold of this should be a separate patch).

Thanks!

								Honza

> 
>  fs/isofs/inode.c       |  9 +++++----
>  fs/udf/super.c         |  9 +++++----
>  include/linux/parser.h |  1 +
>  lib/parser.c           | 44 +++++++++++++++++++++++++++++++++-----------
>  4 files changed, 44 insertions(+), 19 deletions(-)
> 
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v2 0/3] handle large user and group ID for isofs and udf

[Matthew Wilcox]

On Thu, Jan 28, 2021 at 11:55:01AM +0100, Jan Kara wrote:
> On Thu 28-01-21 15:12:27, bingjingc wrote:
> > From: BingJing Chang <bingjingc@synology.com>
> > 
> > The uid/gid (unsigned int) of a domain user may be larger than INT_MAX.
> > The parse_options of isofs and udf will return 0, and mount will fail
> > with -EINVAL. These patches try to handle large user and group ID.
> > 
> > BingJing Chang (3):
> >   parser: add unsigned int parser
> >   isofs: handle large user and group ID
> >   udf: handle large user and group ID
> 
> Thanks for your patches! Just two notes:
> 
> 1) I don't think Matthew Wilcox gave you his Reviewed-by tag (at least I
> didn't see such email). Generally the rule is that the developer has to
> explicitely write in his email that you can attach his Reviewed-by tag for
> it to be valid.

Right, I didn't.

Looking at fuse, they deleted their copy of match_uint
in favour of switching to the fs_parameter_spec (commit
c30da2e981a703c6b1d49911511f7ade8dac20be) and I wonder if isofs & udf
shouldn't receive the same attention.

Re: [PATCH v2 0/3] handle large user and group ID for isofs and udf

[bingjing chang]

Hi Jan & Matthew,

Thank you for your kind notices and comments. Please see my message
below.

bingjingc <bingjingc@synology.com> 於 2021年1月29日 週五 下午1:06寫道：
>
> [loop bxxxjxxg@gmail.com] in order to reply in plain-text
>
> Matthew Wilcox <willy@infradead.org> 於 2021-01-28 22:20 寫道：
>
> On Thu, Jan 28, 2021 at 11:55:01AM +0100, Jan Kara wrote:
> > On Thu 28-01-21 15:12:27, bingjingc wrote:
> > > From: BingJing Chang <bingjingc@synology.com>
> > >
> > > The uid/gid (unsigned int) of a domain user may be larger than INT_MAX.
> > > The parse_options of isofs and udf will return 0, and mount will fail
> > > with -EINVAL. These patches try to handle large user and group ID.
> > >
> > > BingJing Chang (3):
> > >   parser: add unsigned int parser
> > >   isofs: handle large user and group ID
> > >   udf: handle large user and group ID
> >
> > Thanks for your patches! Just two notes:
> >
> > 1) I don't think Matthew Wilcox gave you his Reviewed-by tag (at least I
> > didn't see such email). Generally the rule is that the developer has to
> > explicitely write in his email that you can attach his Reviewed-by tag for
> > it to be valid.
>
> Right, I didn't.

Sorry, I don't know how Reviewed-by tag works in emailing code review
procedures. Thank for talking me the rule. I drop Matthew's Reviewed-by
tag in the third patch.

>
> Looking at fuse, they deleted their copy of match_uint
> in favour of switching to the fs_parameter_spec (commit
> c30da2e981a703c6b1d49911511f7ade8dac20be) and I wonder if isofs & udf
> shouldn't receive the same attention.

That's true. New mount API can handle uint_32 by fs_parse.
It may take a little larger coding and review works converting isofs and udf
to use the new mount API. And we also want these fixes can be also applied
back to previous stable kernels. So we'd like to submit this patch to solve the
iso image mounting bugs first.


Thanks,
BingJing