| Message ID | 7f2987dd3b8eace03dece289d35011ef65170c11.1501124092.git.ernesto.mnd.fernandez@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu 27-07-17 00:13:54, Ernesto A. Fernández wrote: > The hfsplus module already has the code to support access control > lists, but there is no corresponding mount option. Add it and keep it > disabled by default. Hum, this seems to be a regression caused by Christoph's ACL rework in 2013. Since then HFS+ acls didn't work. So much for how much they are used... Maybe time to drop the feature when nobody noticed it does not work for 4 years? If people think the functionality should be kept, the patch makes sense so you can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> > --- > Documentation/filesystems/hfsplus.txt | 4 ++++ > fs/hfsplus/hfsplus_fs.h | 1 + > fs/hfsplus/options.c | 15 ++++++++++++++- > fs/hfsplus/super.c | 1 + > 4 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/Documentation/filesystems/hfsplus.txt b/Documentation/filesystems/hfsplus.txt > index 59f7569..83670ac 100644 > --- a/Documentation/filesystems/hfsplus.txt > +++ b/Documentation/filesystems/hfsplus.txt > @@ -43,6 +43,10 @@ When mounting an HFSPlus filesystem, the following options are accepted: > nodecompose > Do not decompose file name characters. > > + acl > + Enable POSIX Access Control Lists support, disabled by default. > + Requires CONFIG_HFSPLUS_FS_POSIX_ACL set in the kernel configuration. > + > force > Used to force write access to volumes that are marked as journalled > or locked. Use at your own risk. > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h > index a3f03b2..8bc78c1 100644 > --- a/fs/hfsplus/hfsplus_fs.h > +++ b/fs/hfsplus/hfsplus_fs.h > @@ -153,6 +153,7 @@ struct hfsplus_sb_info { > struct inode *alloc_file; > struct inode *hidden_dir; > struct nls_table *nls; > + struct super_block *sb; > > /* Runtime variables */ > u32 blockoffset; > diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c > index bb806e5..6f8b5a9 100644 > --- a/fs/hfsplus/options.c > +++ b/fs/hfsplus/options.c > @@ -24,7 +24,7 @@ enum { > opt_part, opt_session, opt_nls, > opt_nodecompose, opt_decompose, > opt_barrier, opt_nobarrier, > - opt_force, opt_err > + opt_force, opt_acl, opt_err > }; > > static const match_table_t tokens = { > @@ -41,6 +41,7 @@ static const match_table_t tokens = { > { opt_barrier, "barrier" }, > { opt_nobarrier, "nobarrier" }, > { opt_force, "force" }, > + { opt_acl, "acl" }, > { opt_err, NULL } > }; > > @@ -195,6 +196,14 @@ int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi) > case opt_force: > set_bit(HFSPLUS_SB_FORCE, &sbi->flags); > break; > + case opt_acl: > +#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL > + sbi->sb->s_flags |= MS_POSIXACL; > + break; > +#else > + pr_err("support for ACL not compiled in!"); > + return 0; > +#endif > default: > return 0; > } > @@ -234,5 +243,9 @@ int hfsplus_show_options(struct seq_file *seq, struct dentry *root) > seq_puts(seq, ",nodecompose"); > if (test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags)) > seq_puts(seq, ",nobarrier"); > +#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL > + if (sbi->sb->s_flags & MS_POSIXACL) > + seq_puts(seq, ",acl"); > +#endif > return 0; > } > diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c > index 67aedf4..258fb86 100644 > --- a/fs/hfsplus/super.c > +++ b/fs/hfsplus/super.c > @@ -389,6 +389,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent) > goto out; > > sb->s_fs_info = sbi; > + sbi->sb = sb; > mutex_init(&sbi->alloc_mutex); > mutex_init(&sbi->vh_mutex); > spin_lock_init(&sbi->work_lock); > -- > 2.1.4 >
On Thu, Jul 27, 2017 at 11:41:47AM +0200, Jan Kara wrote: > On Thu 27-07-17 00:13:54, Ernesto A. Fernández wrote: > > The hfsplus module already has the code to support access control > > lists, but there is no corresponding mount option. Add it and keep it > > disabled by default. > > Hum, this seems to be a regression caused by Christoph's ACL rework in > 2013. Ooops. > Since then HFS+ acls didn't work. So much for how much they are > used... Maybe time to drop the feature when nobody noticed it does not work > for 4 years? Especially given that they aren't actually compatible with MacOS I'm all in favor of dropping the support.
On Thu, Jul 27, 2017 at 11:41:47AM +0200, Jan Kara wrote: > On Thu 27-07-17 00:13:54, Ernesto A. Fernández wrote: > > The hfsplus module already has the code to support access control > > lists, but there is no corresponding mount option. Add it and keep it > > disabled by default. > > Hum, this seems to be a regression caused by Christoph's ACL rework in > 2013. Since then HFS+ acls didn't work. So much for how much they are > used... Maybe time to drop the feature when nobody noticed it does not work > for 4 years? I'm not sure the lack of use is a problem of ACLs in particular. I quickly ran into trouble with extended attributes as well, so my guess is that all of the module gets very limited use. I'll try and send some patches as soon as I can.
diff --git a/Documentation/filesystems/hfsplus.txt b/Documentation/filesystems/hfsplus.txt index 59f7569..83670ac 100644 --- a/Documentation/filesystems/hfsplus.txt +++ b/Documentation/filesystems/hfsplus.txt @@ -43,6 +43,10 @@ When mounting an HFSPlus filesystem, the following options are accepted: nodecompose Do not decompose file name characters. + acl + Enable POSIX Access Control Lists support, disabled by default. + Requires CONFIG_HFSPLUS_FS_POSIX_ACL set in the kernel configuration. + force Used to force write access to volumes that are marked as journalled or locked. Use at your own risk. diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h index a3f03b2..8bc78c1 100644 --- a/fs/hfsplus/hfsplus_fs.h +++ b/fs/hfsplus/hfsplus_fs.h @@ -153,6 +153,7 @@ struct hfsplus_sb_info { struct inode *alloc_file; struct inode *hidden_dir; struct nls_table *nls; + struct super_block *sb; /* Runtime variables */ u32 blockoffset; diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c index bb806e5..6f8b5a9 100644 --- a/fs/hfsplus/options.c +++ b/fs/hfsplus/options.c @@ -24,7 +24,7 @@ enum { opt_part, opt_session, opt_nls, opt_nodecompose, opt_decompose, opt_barrier, opt_nobarrier, - opt_force, opt_err + opt_force, opt_acl, opt_err }; static const match_table_t tokens = { @@ -41,6 +41,7 @@ static const match_table_t tokens = { { opt_barrier, "barrier" }, { opt_nobarrier, "nobarrier" }, { opt_force, "force" }, + { opt_acl, "acl" }, { opt_err, NULL } }; @@ -195,6 +196,14 @@ int hfsplus_parse_options(char *input, struct hfsplus_sb_info *sbi) case opt_force: set_bit(HFSPLUS_SB_FORCE, &sbi->flags); break; + case opt_acl: +#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL + sbi->sb->s_flags |= MS_POSIXACL; + break; +#else + pr_err("support for ACL not compiled in!"); + return 0; +#endif default: return 0; } @@ -234,5 +243,9 @@ int hfsplus_show_options(struct seq_file *seq, struct dentry *root) seq_puts(seq, ",nodecompose"); if (test_bit(HFSPLUS_SB_NOBARRIER, &sbi->flags)) seq_puts(seq, ",nobarrier"); +#ifdef CONFIG_HFSPLUS_FS_POSIX_ACL + if (sbi->sb->s_flags & MS_POSIXACL) + seq_puts(seq, ",acl"); +#endif return 0; } diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c index 67aedf4..258fb86 100644 --- a/fs/hfsplus/super.c +++ b/fs/hfsplus/super.c @@ -389,6 +389,7 @@ static int hfsplus_fill_super(struct super_block *sb, void *data, int silent) goto out; sb->s_fs_info = sbi; + sbi->sb = sb; mutex_init(&sbi->alloc_mutex); mutex_init(&sbi->vh_mutex); spin_lock_init(&sbi->work_lock);
The hfsplus module already has the code to support access control lists, but there is no corresponding mount option. Add it and keep it disabled by default. Signed-off-by: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com> --- Documentation/filesystems/hfsplus.txt | 4 ++++ fs/hfsplus/hfsplus_fs.h | 1 + fs/hfsplus/options.c | 15 ++++++++++++++- fs/hfsplus/super.c | 1 + 4 files changed, 20 insertions(+), 1 deletion(-)