On Tue, Aug 24, 2021 at 05:13:01PM +0900, Namjae Jeon wrote:
> 2021-08-24 0:13 GMT+09:00, Christian Brauner <brauner@xxxxxxxxxx>:
> > From: Christian Brauner <christian.brauner@xxxxxxxxxx>
> >
> > It's not obvious why subauth 0 would be excluded from translation. This
> > would lead to wrong results whenever a non-identity idmapping is used.
> >
> > Cc: Steve French <stfrench@xxxxxxxxxxxxx>
> > Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> > Cc: Namjae Jeon <namjae.jeon@xxxxxxxxxxx>
> > Cc: Hyunchul Lee <hyc.lee@xxxxxxxxx>
> > Cc: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
> > Cc: linux-cifs@xxxxxxxxxxxxxxx
> > Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> > ---
> > fs/ksmbd/smbacl.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c
> > index 3307ca776eb1..0d269b28f163 100644
> > --- a/fs/ksmbd/smbacl.c
> > +++ b/fs/ksmbd/smbacl.c
> > @@ -274,7 +274,7 @@ static int sid_to_id(struct user_namespace *user_ns,
> > uid_t id;
> >
> > id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
> > - if (id > 0) {
> > + if (id >= 0) {
> > uid = make_kuid(user_ns, id);
> > if (uid_valid(uid) && kuid_has_mapping(user_ns, uid)) {
> > fattr->cf_uid = uid;
> > @@ -286,9 +286,9 @@ static int sid_to_id(struct user_namespace *user_ns,
> > gid_t id;
> >
> > id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
> > - if (id > 0) {
> > gid = make_kgid(user_ns, id);
> > if (gid_valid(gid) && kgid_has_mapping(user_ns, gid)) {
> > + if (id >= 0) {
> Checkpatch.pl give warning messages like the following :
>
> WARNING: suspect code indent for conditional statements (24, 16)
> #110: FILE: fs/ksmbd/smbacl.c:290:
> if (gid_valid(gid) && kgid_has_mapping(user_ns, gid)) {
> + if (id >= 0) {
>
> WARNING: suspect code indent for conditional statements (16, 32)
> #111: FILE: fs/ksmbd/smbacl.c:291:
> + if (id >= 0) {
> fattr->cf_gid = gid;
>
> With 7th patch, it shouldn't be a problem, but this patch itself seems
> to have a problem.
> I will directly update it like this if you don't mind :
>
> id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]);
> - if (id > 0) {
> + if (id >= 0) {
> gid = make_kgid(user_ns, id);
> if (gid_valid(gid) && kgid_has_mapping(user_ns, gid)) {
> fattr->cf_gid = gid;
> rc = 0;
> }
>
Sounds good. Thanks!
Christian
