2021-01-28 07:14:55

by bingjingc

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

From: BingJing Chang <[email protected]>

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


2021-01-28 10:56:47

by Jan Kara

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

On Thu 28-01-21 15:12:27, bingjingc wrote:
> From: BingJing Chang <[email protected]>
>
> 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 <[email protected]>
SUSE Labs, CR

2021-01-28 14:21:52

by Matthew Wilcox

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

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 <[email protected]>
> >
> > 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.

2021-01-29 05:35:03

by bingjing chang

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

Hi Jan & Matthew,

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

bingjingc <[email protected]> 於 2021年1月29日 週五 下午1:06寫道:
>
> [loop [email protected]] in order to reply in plain-text
>
> Matthew Wilcox <[email protected]> 於 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 <[email protected]>
> > >
> > > 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