linux-kernel - Re: [PATCH] hfsplus: Fix bug causing custom uid and gid being unable to be assigned with mount

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20EA622D-12B9-49B6-8564-C8EE2A2329D2@dubeyko.com>
Date:   Tue, 6 Dec 2022 10:56:56 -0800
From:   Viacheslav Dubeyko <slava@...eyko.com>
To:     Aditya Garg <gargaditya08@...e.com>
Cc:     "willy@...radead.org" <willy@...radead.org>,
        "ira.weiny@...el.com" <ira.weiny@...el.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "bvanassche@....org" <bvanassche@....org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "songmuchun@...edance.com" <songmuchun@...edance.com>,
        "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hfsplus: Fix bug causing custom uid and gid being unable
 to be assigned with mount



> On Dec 6, 2022, at 12:49 AM, Aditya Garg <gargaditya08@...e.com> wrote:
> 
>> 
>> Well initially I when I tried to investigate what’s wrong, and found that the old logic was the culprit, I did some logging to see what exactly was wrong. The log patch is here btw :- https://github.com/AdityaGarg8/linux/commit/f668fb012f595d83053020b88b9439c295b4dc21
>> 
>> So I saw that the old logic was always false, no matter whether I mounted with uid or not.
>> 
>> I tried to see what makes this true, but couldn't succeed. So, I thought of a simpler approach and changed the logic itself.
>> 
>> To be honest, I dunno what is the old logic for. Maybe instead of completely removing the old logic, I could use an OR?
>> 
>> If you think its more logical, I can make this change :-
>> 
>> -	if (!i_gid_read(inode) && !mode)
>> +	if ((test_bit(HFSPLUS_SB_UID, &sbi->flags)) || (!i_uid_read(inode) && !mode))
>> 
>> Thanks
>> Aditya
>> 
>> 
> 
> I continuation with this message, I also think the bits should be set only if (!uid_valid(sbi->uid) is false, else the bits may be set even if UID is invalid? So, do you think the change given below should be good for this?
> 
> diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c
> index 047e05c57..c94a58762 100644
> --- a/fs/hfsplus/options.c
> +++ b/fs/hfsplus/options.c
> @@ -140,6 +140,8 @@ int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi)
> 			if (!uid_valid(sbi->uid)) {
> 				pr_err("invalid uid specified\n");
> 				return 0;
> +			} else {
> +				set_bit(HFSPLUS_SB_UID, &sbi->flags);
> 			}
> 			break;
> 		case opt_gid:
> @@ -151,6 +153,8 @@ int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi)
> 			if (!gid_valid(sbi->gid)) {
> 				pr_err("invalid gid specified\n");
> 				return 0;
> +			} else {
> +				set_bit(HFSPLUS_SB_GID, &sbi->flags);
> 			}
> 			break;
> 		case opt_part:

Looks reasonably well. I believe it’s better fix.

Thanks,
Slava.