Re: [PATCH 06/11] ksmbd: fix subauth 0 handling in sid_to_id()

[Namjae Jeon <linkinjeon@kernel.org>]

2021-08-24 0:13 GMT+09:00, Christian Brauner <brauner@kernel.org>:
> From: Christian Brauner <christian.brauner@ubuntu.com>
>
> 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@microsoft.com>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Namjae Jeon <namjae.jeon@samsung.com>
> Cc: Hyunchul Lee <hyc.lee@gmail.com>
> Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
> Cc: linux-cifs@vger.kernel.org
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> ---
>  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;
                        }

Thanks!
				fattr->cf_gid = gid;
>  				rc = 0;
>  			}
> --
> 2.30.2
>
>