Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Content-Type: text/plain;
        charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.1\))
Subject: Re: [PATCH] hfsplus: Fix bug causing custom uid and gid being unable
 to be assigned with mount
From:   Viacheslav Dubeyko <slava@dubeyko.com>
In-Reply-To: <1D7AAEE4-9603-43A4-B89D-6F791EDCB929@live.com>
Date:   Mon, 5 Dec 2022 16:35:36 -0800
Cc:     "willy@infradead.org" <willy@infradead.org>,
        "ira.weiny@intel.com" <ira.weiny@intel.com>,
        "axboe@kernel.dk" <axboe@kernel.dk>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "keescook@chromium.org" <keescook@chromium.org>,
        "songmuchun@bytedance.com" <songmuchun@bytedance.com>,
        "torvalds@linux-foundation.org" <torvalds@linux-foundation.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <A2B962C1-AD33-413D-B64A-CD179AFBEA8D@dubeyko.com>
References: <93F5197D-2B61-4129-B5D4-771934F70577@live.com>
 <55A80630-60FB-44BE-9628-29104AB8A7D0@dubeyko.com>
 <1D7AAEE4-9603-43A4-B89D-6F791EDCB929@live.com>
To:     Aditya Garg <gargaditya08@live.com>
Precedence: bulk


> On Dec 2, 2022, at 11:56 PM, Aditya Garg <gargaditya08@live.com> =
wrote:
>=20
>=20
>> Also, what if we mounted
>> file system without specifying the UID/GID, then what UID/GID will be =
returned by
>> your logic?
>=20
> So this case is if I run =E2=80=9Csudo mount /dev/sda1 /mnt=E2=80=9D
>=20
> Here the driver will not do any spoofing, and the real owners of the =
files shall be displayed. Thus running =E2=80=9Cls -l=E2=80=9D on a =
mounted partition without specifying UID/GID, files written by macOS =
shall be shown as 99 as the owner, iPadOS as 501, and if any file was =
written on Linux, the user who wrote it will be the owner.
>=20
> If the user/group of any file was changed using chown, then the new =
user/group of the file will be displayed.

My question is much more simple.

diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index aeab83ed1..4d1077db8 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -192,11 +192,11 @@ static void hfsplus_get_perms(struct inode *inode,
	mode =3D be16_to_cpu(perms->mode);

	i_uid_write(inode, be32_to_cpu(perms->owner));
-	if (!i_uid_read(inode) && !mode)
+	if (test_bit(HFSPLUS_SB_UID, &sbi->flags))
		inode->i_uid =3D sbi->uid;

	i_gid_write(inode, be32_to_cpu(perms->group));
-	if (!i_gid_read(inode) && !mode)
+	if (test_bit(HFSPLUS_SB_GID, &sbi->flags))
		inode->i_gid =3D sbi->gid;

Before this change, logic called i_uid_read(inode) and checked mode.
Now, we check only HFSPLUS_SB_UID/HFSPLUS_SB_GID flags.
So, if we mount HFS+ volume by =E2=80=9Csudo mount /dev/sda1 /mnt=E2=80=9D=
, then
HFSPLUS_SB_UID and HFSPLUS_SB_GID flags will be unset.
And current logic will do nothing. Is it correct logic? Maybe, we need
to use sbi->uid/gid if flag(s)HFSPLUS_SB_UID/HFSPLUS_SB_GID are set.
And if not, then to use old logic. Am I correct here? Or am I still =
missing
something here?

Thanks,
Slava.