> On Dec 6, 2022, at 12:49 AM, Aditya Garg <gargaditya08@xxxxxxxx> 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.
