All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ANDROID: binderfs: add capabilities support
@ 2021-07-07 16:24 Carlos Llamas
  2021-07-07 16:59 ` Greg Kroah-Hartman
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Carlos Llamas @ 2021-07-07 16:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner
  Cc: Joel Fernandes, Steven Moreland, kernel-team, linux-kernel,
	Carlos Llamas

Provide userspace with a mechanism to discover binder driver
capabilities to refrain from using these unsupported features
in the first place. Note that older capabilities are assumed
to be supported and only new ones will be added.

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index e80ba93c62a9..f793887f6dc8 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -58,6 +58,10 @@ enum binderfs_stats_mode {
 	binderfs_stats_mode_global,
 };
 
+struct binder_capabilities {
+	bool oneway_spam;
+};
+
 static const struct constant_table binderfs_param_stats[] = {
 	{ "global", binderfs_stats_mode_global },
 	{}
@@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
 	{}
 };
 
+static struct binder_capabilities binder_caps = {
+	.oneway_spam = true,
+};
+
 static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
 {
 	return sb->s_fs_info;
@@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
 	return dentry;
 }
 
+static int binder_caps_show(struct seq_file *m, void *unused)
+{
+	bool *cap = m->private;
+
+	seq_printf(m, "%d\n", *cap);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(binder_caps);
+
+static int init_binder_caps(struct super_block *sb)
+{
+	struct dentry *dentry, *root;
+	int ret = 0;
+
+	root = binderfs_create_dir(sb->s_root, "caps");
+	if (IS_ERR(root)) {
+		ret = PTR_ERR(root);
+		goto out;
+	}
+
+	dentry = binderfs_create_file(root, "oneway_spam",
+				      &binder_caps_fops,
+				      &binder_caps.oneway_spam);
+	if (IS_ERR(dentry)) {
+		ret = PTR_ERR(dentry);
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
 static int init_binder_logs(struct super_block *sb)
 {
 	struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir;
@@ -723,6 +764,10 @@ static int binderfs_fill_super(struct super_block *sb, struct fs_context *fc)
 			name++;
 	}
 
+	ret = init_binder_caps(sb);
+	if (ret)
+		return ret;
+
 	if (info->mount_opts.stats_mode == binderfs_stats_mode_global)
 		return init_binder_logs(sb);
 
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ANDROID: binderfs: add capabilities support
  2021-07-07 16:24 [PATCH] ANDROID: binderfs: add capabilities support Carlos Llamas
@ 2021-07-07 16:59 ` Greg Kroah-Hartman
  2021-07-08 16:33   ` Carlos Llamas
  2021-07-09  8:39   ` Christian Brauner
  2021-07-09  8:55 ` Christian Brauner
  2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
  2 siblings, 2 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2021-07-07 16:59 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Christian Brauner, Joel Fernandes, Steven Moreland, kernel-team,
	linux-kernel

On Wed, Jul 07, 2021 at 04:24:19PM +0000, Carlos Llamas wrote:
> Provide userspace with a mechanism to discover binder driver
> capabilities to refrain from using these unsupported features
> in the first place. Note that older capabilities are assumed
> to be supported and only new ones will be added.

What defines "new" vs. "old"?  Where was the line drawn?

> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
>  drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index e80ba93c62a9..f793887f6dc8 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -58,6 +58,10 @@ enum binderfs_stats_mode {
>  	binderfs_stats_mode_global,
>  };
>  
> +struct binder_capabilities {
> +	bool oneway_spam;
> +};
> +
>  static const struct constant_table binderfs_param_stats[] = {
>  	{ "global", binderfs_stats_mode_global },
>  	{}
> @@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
>  	{}
>  };
>  
> +static struct binder_capabilities binder_caps = {
> +	.oneway_spam = true,
> +};
> +
>  static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
>  {
>  	return sb->s_fs_info;
> @@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
>  	return dentry;
>  }
>  
> +static int binder_caps_show(struct seq_file *m, void *unused)
> +{
> +	bool *cap = m->private;
> +
> +	seq_printf(m, "%d\n", *cap);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(binder_caps);
> +
> +static int init_binder_caps(struct super_block *sb)
> +{
> +	struct dentry *dentry, *root;
> +	int ret = 0;
> +
> +	root = binderfs_create_dir(sb->s_root, "caps");
> +	if (IS_ERR(root)) {
> +		ret = PTR_ERR(root);
> +		goto out;
> +	}
> +
> +	dentry = binderfs_create_file(root, "oneway_spam",
> +				      &binder_caps_fops,
> +				      &binder_caps.oneway_spam);
> +	if (IS_ERR(dentry)) {
> +		ret = PTR_ERR(dentry);
> +		goto out;

If this fails, you still report that an error happened, yet you do not
remove the directory?  Is that intended?

And where is this new file documented?  Where are the existing binderfs
files documented?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ANDROID: binderfs: add capabilities support
  2021-07-07 16:59 ` Greg Kroah-Hartman
@ 2021-07-08 16:33   ` Carlos Llamas
  2021-07-09  8:39   ` Christian Brauner
  1 sibling, 0 replies; 12+ messages in thread
From: Carlos Llamas @ 2021-07-08 16:33 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Arve Hjønnevåg, Todd Kjos, Martijn Coenen,
	Christian Brauner, Joel Fernandes, Steven Moreland, kernel-team,
	linux-kernel

On Wed, Jul 7, 2021 at 10:59 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jul 07, 2021 at 04:24:19PM +0000, Carlos Llamas wrote:
> > Provide userspace with a mechanism to discover binder driver
> > capabilities to refrain from using these unsupported features
> > in the first place. Note that older capabilities are assumed
> > to be supported and only new ones will be added.
>
> What defines "new" vs. "old"?  Where was the line drawn?

Any feature after "one way spam" may be added to the capabilities.
I'll amend the commit message to indicate this.

>
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > ---
> >  drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index e80ba93c62a9..f793887f6dc8 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -58,6 +58,10 @@ enum binderfs_stats_mode {
> >       binderfs_stats_mode_global,
> >  };
> >
> > +struct binder_capabilities {
> > +     bool oneway_spam;
> > +};
> > +
> >  static const struct constant_table binderfs_param_stats[] = {
> >       { "global", binderfs_stats_mode_global },
> >       {}
> > @@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
> >       {}
> >  };
> >
> > +static struct binder_capabilities binder_caps = {
> > +     .oneway_spam = true,
> > +};
> > +
> >  static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
> >  {
> >       return sb->s_fs_info;
> > @@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
> >       return dentry;
> >  }
> >
> > +static int binder_caps_show(struct seq_file *m, void *unused)
> > +{
> > +     bool *cap = m->private;
> > +
> > +     seq_printf(m, "%d\n", *cap);
> > +
> > +     return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(binder_caps);
> > +
> > +static int init_binder_caps(struct super_block *sb)
> > +{
> > +     struct dentry *dentry, *root;
> > +     int ret = 0;
> > +
> > +     root = binderfs_create_dir(sb->s_root, "caps");
> > +     if (IS_ERR(root)) {
> > +             ret = PTR_ERR(root);
> > +             goto out;
> > +     }
> > +
> > +     dentry = binderfs_create_file(root, "oneway_spam",
> > +                                   &binder_caps_fops,
> > +                                   &binder_caps.oneway_spam);
> > +     if (IS_ERR(dentry)) {
> > +             ret = PTR_ERR(dentry);
> > +             goto out;
>
> If this fails, you still report that an error happened, yet you do not
> remove the directory?  Is that intended?

Propagating an error in this case will kill the super block along with the
capabilities entries. At a user level the binderfs will fail to be mounted,
so anything created up until this point is removed.

>
> And where is this new file documented?  Where are the existing binderfs
> files documented?

oh! we do have "Documentation/admin-guide/binderfs.rst". I'll add a
brief explanation of these new capabilities files there.

>
> thanks,
>
> greg k-h

thanks,
carlos llamas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ANDROID: binderfs: add capabilities support
  2021-07-07 16:59 ` Greg Kroah-Hartman
  2021-07-08 16:33   ` Carlos Llamas
@ 2021-07-09  8:39   ` Christian Brauner
  1 sibling, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-07-09  8:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Carlos Llamas, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Wed, Jul 07, 2021 at 06:59:46PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 07, 2021 at 04:24:19PM +0000, Carlos Llamas wrote:
> > Provide userspace with a mechanism to discover binder driver
> > capabilities to refrain from using these unsupported features
> > in the first place. Note that older capabilities are assumed
> > to be supported and only new ones will be added.
> 
> What defines "new" vs. "old"?  Where was the line drawn?
> 
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > ---
> >  drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> > 
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index e80ba93c62a9..f793887f6dc8 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -58,6 +58,10 @@ enum binderfs_stats_mode {
> >  	binderfs_stats_mode_global,
> >  };
> >  
> > +struct binder_capabilities {
> > +	bool oneway_spam;
> > +};
> > +
> >  static const struct constant_table binderfs_param_stats[] = {
> >  	{ "global", binderfs_stats_mode_global },
> >  	{}
> > @@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
> >  	{}
> >  };
> >  
> > +static struct binder_capabilities binder_caps = {
> > +	.oneway_spam = true,
> > +};
> > +
> >  static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
> >  {
> >  	return sb->s_fs_info;
> > @@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
> >  	return dentry;
> >  }
> >  
> > +static int binder_caps_show(struct seq_file *m, void *unused)
> > +{
> > +	bool *cap = m->private;
> > +
> > +	seq_printf(m, "%d\n", *cap);
> > +
> > +	return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(binder_caps);
> > +
> > +static int init_binder_caps(struct super_block *sb)
> > +{
> > +	struct dentry *dentry, *root;
> > +	int ret = 0;
> > +
> > +	root = binderfs_create_dir(sb->s_root, "caps");
> > +	if (IS_ERR(root)) {
> > +		ret = PTR_ERR(root);
> > +		goto out;
> > +	}
> > +
> > +	dentry = binderfs_create_file(root, "oneway_spam",
> > +				      &binder_caps_fops,
> > +				      &binder_caps.oneway_spam);
> > +	if (IS_ERR(dentry)) {
> > +		ret = PTR_ERR(dentry);
> > +		goto out;
> 
> If this fails, you still report that an error happened, yet you do not
> remove the directory?  Is that intended?
> 
> And where is this new file documented?  Where are the existing binderfs
> files documented?

When I wrote it I added documentation to:

Documentation/admin-guide/binderfs.rst

So the new caps directory and file should be documented there.

I would also suggest to add a simple test reading this new caps
directory and the new file in there to:

tools/testing/selftests/filesystems/binderfs_test.c

Christian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ANDROID: binderfs: add capabilities support
  2021-07-07 16:24 [PATCH] ANDROID: binderfs: add capabilities support Carlos Llamas
  2021-07-07 16:59 ` Greg Kroah-Hartman
@ 2021-07-09  8:55 ` Christian Brauner
  2021-07-09 16:32   ` Carlos Llamas
  2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
  2 siblings, 1 reply; 12+ messages in thread
From: Christian Brauner @ 2021-07-09  8:55 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Wed, Jul 07, 2021 at 04:24:19PM +0000, Carlos Llamas wrote:
> Provide userspace with a mechanism to discover binder driver
> capabilities to refrain from using these unsupported features

Hey Carlos,

The model will be one file per feature?

Instead of calling the directory "caps" should this maybe be called
"features"? I'm not fuzzed about it and if you want to keep "caps"
that's fine. The term is just a bit overused and makes me think of other
things than this.

> in the first place. Note that older capabilities are assumed
> to be supported and only new ones will be added.

What if you ever want to deprecate one? :)

> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---
>  drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index e80ba93c62a9..f793887f6dc8 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -58,6 +58,10 @@ enum binderfs_stats_mode {
>  	binderfs_stats_mode_global,
>  };
>  
> +struct binder_capabilities {
> +	bool oneway_spam;
> +};
> +
>  static const struct constant_table binderfs_param_stats[] = {
>  	{ "global", binderfs_stats_mode_global },
>  	{}
> @@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
>  	{}
>  };
>  
> +static struct binder_capabilities binder_caps = {
> +	.oneway_spam = true,

I know this is the oneway spam _detection_ feature but this file makes
it sound like the binder driver has the capability to generate one-way
spam. :) Maybe name at least name the file "oneway_spam_detection".

> +};
> +
>  static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
>  {
>  	return sb->s_fs_info;
> @@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
>  	return dentry;
>  }
>  
> +static int binder_caps_show(struct seq_file *m, void *unused)
> +{
> +	bool *cap = m->private;
> +
> +	seq_printf(m, "%d\n", *cap);
> +
> +	return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(binder_caps);
> +
> +static int init_binder_caps(struct super_block *sb)

You can drop the goto here and just always return directly.

> +{
> +	struct dentry *dentry, *root;

Please name this "dir" instead of "root". "root" is conventionally used
for sb->s_root and especially here in this file I only ever used it to
indicate s_root.

> +	int ret = 0;
> +
> +	root = binderfs_create_dir(sb->s_root, "caps");
> +	if (IS_ERR(root)) {
> +		ret = PTR_ERR(root);

	return PTR_ERR(root);

> +		goto out;
> +	}
> +
> +	dentry = binderfs_create_file(root, "oneway_spam",
> +				      &binder_caps_fops,
> +				      &binder_caps.oneway_spam);
> +	if (IS_ERR(dentry)) {
> +		ret = PTR_ERR(dentry);

	return PTR_ERR(root);

> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> +
>  static int init_binder_logs(struct super_block *sb)
>  {
>  	struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir;
> @@ -723,6 +764,10 @@ static int binderfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  			name++;
>  	}
>  
> +	ret = init_binder_caps(sb);
> +	if (ret)
> +		return ret;
> +
>  	if (info->mount_opts.stats_mode == binderfs_stats_mode_global)
>  		return init_binder_logs(sb);
>  
> -- 
> 2.32.0.93.g670b81a890-goog
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] ANDROID: binderfs: add capabilities support
  2021-07-09  8:55 ` Christian Brauner
@ 2021-07-09 16:32   ` Carlos Llamas
  0 siblings, 0 replies; 12+ messages in thread
From: Carlos Llamas @ 2021-07-09 16:32 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Fri, Jul 9, 2021 at 2:56 AM Christian Brauner
<christian.brauner@ubuntu.com> wrote:
>
> On Wed, Jul 07, 2021 at 04:24:19PM +0000, Carlos Llamas wrote:
> > Provide userspace with a mechanism to discover binder driver
> > capabilities to refrain from using these unsupported features
>
> Hey Carlos,
>
> The model will be one file per feature?

Yes. I dropped a previous single bitmask file idea per Greg's suggestion.
The file per feature improves on a number of areas such as feature count
limit, readability and it's easier to manage (add/remove features).

>
> Instead of calling the directory "caps" should this maybe be called
> "features"? I'm not fuzzed about it and if you want to keep "caps"
> that's fine. The term is just a bit overused and makes me think of other
> things than this.

I have no problems switching over to "features".

>
> > in the first place. Note that older capabilities are assumed
> > to be supported and only new ones will be added.
>
> What if you ever want to deprecate one? :)

If the file for a feature doesn't exist then such feature is not supported.
So we can avoid creating such file if a feature were to be deprecated.

>
> >
> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
> > ---
> >  drivers/android/binderfs.c | 45 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> > index e80ba93c62a9..f793887f6dc8 100644
> > --- a/drivers/android/binderfs.c
> > +++ b/drivers/android/binderfs.c
> > @@ -58,6 +58,10 @@ enum binderfs_stats_mode {
> >       binderfs_stats_mode_global,
> >  };
> >
> > +struct binder_capabilities {
> > +     bool oneway_spam;
> > +};
> > +
> >  static const struct constant_table binderfs_param_stats[] = {
> >       { "global", binderfs_stats_mode_global },
> >       {}
> > @@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
> >       {}
> >  };
> >
> > +static struct binder_capabilities binder_caps = {
> > +     .oneway_spam = true,
>
> I know this is the oneway spam _detection_ feature but this file makes
> it sound like the binder driver has the capability to generate one-way
> spam. :) Maybe name at least name the file "oneway_spam_detection".

That's true. I'll rename it as suggested.

>
> > +};
> > +
> >  static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
> >  {
> >       return sb->s_fs_info;
> > @@ -583,6 +591,39 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
> >       return dentry;
> >  }
> >
> > +static int binder_caps_show(struct seq_file *m, void *unused)
> > +{
> > +     bool *cap = m->private;
> > +
> > +     seq_printf(m, "%d\n", *cap);
> > +
> > +     return 0;
> > +}
> > +DEFINE_SHOW_ATTRIBUTE(binder_caps);
> > +
> > +static int init_binder_caps(struct super_block *sb)
>
> You can drop the goto here and just always return directly.

I also noticed this and I decided to keep it consistent with init_binder_logs()
structure. But I don't have a strong preference so I'll switch to
early returns.

>
> > +{
> > +     struct dentry *dentry, *root;
>
> Please name this "dir" instead of "root". "root" is conventionally used
> for sb->s_root and especially here in this file I only ever used it to
> indicate s_root.

ok, sounds good.

>
> > +     int ret = 0;
> > +
> > +     root = binderfs_create_dir(sb->s_root, "caps");
> > +     if (IS_ERR(root)) {
> > +             ret = PTR_ERR(root);
>
>         return PTR_ERR(root);
>
> > +             goto out;
> > +     }
> > +
> > +     dentry = binderfs_create_file(root, "oneway_spam",
> > +                                   &binder_caps_fops,
> > +                                   &binder_caps.oneway_spam);
> > +     if (IS_ERR(dentry)) {
> > +             ret = PTR_ERR(dentry);
>
>         return PTR_ERR(root);
>
> > +             goto out;
> > +     }
> > +
> > +out:
> > +     return ret;
> > +}
> > +
> >  static int init_binder_logs(struct super_block *sb)
> >  {
> >       struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir;
> > @@ -723,6 +764,10 @@ static int binderfs_fill_super(struct super_block *sb, struct fs_context *fc)
> >                       name++;
> >       }
> >
> > +     ret = init_binder_caps(sb);
> > +     if (ret)
> > +             return ret;
> > +
> >       if (info->mount_opts.stats_mode == binderfs_stats_mode_global)
> >               return init_binder_logs(sb);
> >
> > --
> > 2.32.0.93.g670b81a890-goog
> >

thanks,
carlos llamas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] binderfs: add support for feature files
  2021-07-07 16:24 [PATCH] ANDROID: binderfs: add capabilities support Carlos Llamas
  2021-07-07 16:59 ` Greg Kroah-Hartman
  2021-07-09  8:55 ` Christian Brauner
@ 2021-07-15  3:18 ` Carlos Llamas
  2021-07-15  3:18   ` [PATCH v2 2/3] docs: binderfs: add section about " Carlos Llamas
                     ` (2 more replies)
  2 siblings, 3 replies; 12+ messages in thread
From: Carlos Llamas @ 2021-07-15  3:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner
  Cc: Joel Fernandes, Steven Moreland, kernel-team, linux-kernel,
	Carlos Llamas

Provide userspace with a mechanism to discover features supported by
the binder driver to refrain from using any unsupported ones in the
first place. Starting with "oneway_spam_detection" only new features
are to be listed under binderfs and all previous ones are assumed to
be supported.

Assuming an instance of binderfs has been mounted at /dev/binderfs,
binder feature files can be found under /dev/binderfs/features/.
Usage example:

  $ mkdir /dev/binderfs
  $ mount -t binder binder /dev/binderfs
  $ cat /dev/binderfs/features/oneway_spam_detection
  1

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 drivers/android/binderfs.c | 39 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index e80ba93c62a9..e3605cdd4335 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -58,6 +58,10 @@ enum binderfs_stats_mode {
 	binderfs_stats_mode_global,
 };
 
+struct binder_features {
+	bool oneway_spam_detection;
+};
+
 static const struct constant_table binderfs_param_stats[] = {
 	{ "global", binderfs_stats_mode_global },
 	{}
@@ -69,6 +73,10 @@ static const struct fs_parameter_spec binderfs_fs_parameters[] = {
 	{}
 };
 
+static struct binder_features binder_features = {
+	.oneway_spam_detection = true,
+};
+
 static inline struct binderfs_info *BINDERFS_SB(const struct super_block *sb)
 {
 	return sb->s_fs_info;
@@ -583,6 +591,33 @@ static struct dentry *binderfs_create_dir(struct dentry *parent,
 	return dentry;
 }
 
+static int binder_features_show(struct seq_file *m, void *unused)
+{
+	bool *feature = m->private;
+
+	seq_printf(m, "%d\n", *feature);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(binder_features);
+
+static int init_binder_features(struct super_block *sb)
+{
+	struct dentry *dentry, *dir;
+
+	dir = binderfs_create_dir(sb->s_root, "features");
+	if (IS_ERR(dir))
+		return PTR_ERR(dir);
+
+	dentry = binderfs_create_file(dir, "oneway_spam_detection",
+				      &binder_features_fops,
+				      &binder_features.oneway_spam_detection);
+	if (IS_ERR(dentry))
+		return PTR_ERR(dentry);
+
+	return 0;
+}
+
 static int init_binder_logs(struct super_block *sb)
 {
 	struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir;
@@ -723,6 +758,10 @@ static int binderfs_fill_super(struct super_block *sb, struct fs_context *fc)
 			name++;
 	}
 
+	ret = init_binder_features(sb);
+	if (ret)
+		return ret;
+
 	if (info->mount_opts.stats_mode == binderfs_stats_mode_global)
 		return init_binder_logs(sb);
 
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] docs: binderfs: add section about feature files
  2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
@ 2021-07-15  3:18   ` Carlos Llamas
  2021-07-15  8:08     ` Christian Brauner
  2021-07-15  3:18   ` [PATCH v2 3/3] selftests/binderfs: add test for " Carlos Llamas
  2021-07-15  8:07   ` [PATCH v2 1/3] binderfs: add support " Christian Brauner
  2 siblings, 1 reply; 12+ messages in thread
From: Carlos Llamas @ 2021-07-15  3:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner
  Cc: Joel Fernandes, Steven Moreland, kernel-team, linux-kernel,
	Carlos Llamas

Document how binder feature files can be used to determine whether a
feature is supported by the binder driver. "oneway_spam_detection" is
used as an example as it is the first available feature file.

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 Documentation/admin-guide/binderfs.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/admin-guide/binderfs.rst b/Documentation/admin-guide/binderfs.rst
index 199d84314a14..41a4db00df8d 100644
--- a/Documentation/admin-guide/binderfs.rst
+++ b/Documentation/admin-guide/binderfs.rst
@@ -72,3 +72,16 @@ that the `rm() <rm_>`_ tool can be used to delete them. Note that the
 ``binder-control`` device cannot be deleted since this would make the binderfs
 instance unusable.  The ``binder-control`` device will be deleted when the
 binderfs instance is unmounted and all references to it have been dropped.
+
+Binder features
+---------------
+
+Assuming an instance of binderfs has been mounted at ``/dev/binderfs``, the
+features supported by the binder driver can be located under
+``/dev/binderfs/features/``. The presence of individual files can be tested
+to determine whether a particular feature is supported by the driver.
+
+Example::
+
+        cat /dev/binderfs/features/oneway_spam_detection
+        1
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] selftests/binderfs: add test for feature files
  2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
  2021-07-15  3:18   ` [PATCH v2 2/3] docs: binderfs: add section about " Carlos Llamas
@ 2021-07-15  3:18   ` Carlos Llamas
  2021-07-15  8:09     ` Christian Brauner
  2021-07-15  8:07   ` [PATCH v2 1/3] binderfs: add support " Christian Brauner
  2 siblings, 1 reply; 12+ messages in thread
From: Carlos Llamas @ 2021-07-15  3:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner
  Cc: Joel Fernandes, Steven Moreland, kernel-team, linux-kernel,
	Carlos Llamas

Verify that feature files are created successfully after mounting a
binderfs instance. Note that only "oneway_spam_detection" feature is
tested with this patch as it is currently the only feature listed.

Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 .../filesystems/binderfs/binderfs_test.c        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
index 477cbb042f5b..0315955ff0f4 100644
--- a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
+++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c
@@ -62,6 +62,9 @@ static int __do_binderfs_test(struct __test_metadata *_metadata)
 	struct binder_version version = { 0 };
 	char binderfs_mntpt[] = P_tmpdir "/binderfs_XXXXXX",
 		device_path[sizeof(P_tmpdir "/binderfs_XXXXXX/") + BINDERFS_MAX_NAME];
+	static const char * const binder_features[] = {
+		"oneway_spam_detection",
+	};
 
 	change_mountns(_metadata);
 
@@ -150,6 +153,20 @@ static int __do_binderfs_test(struct __test_metadata *_metadata)
 	}
 
 	/* success: binder-control device removal failed as expected */
+
+	for (int i = 0; i < ARRAY_SIZE(binder_features); i++) {
+		snprintf(device_path, sizeof(device_path), "%s/features/%s",
+			 binderfs_mntpt, binder_features[i]);
+		fd = open(device_path, O_CLOEXEC | O_RDONLY);
+		EXPECT_GE(fd, 0) {
+			TH_LOG("%s - Failed to open binder feature: %s",
+				strerror(errno), binder_features[i]);
+			goto umount;
+		}
+		close(fd);
+	}
+
+	/* success: binder feature files found */
 	result = 0;
 
 umount:
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] binderfs: add support for feature files
  2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
  2021-07-15  3:18   ` [PATCH v2 2/3] docs: binderfs: add section about " Carlos Llamas
  2021-07-15  3:18   ` [PATCH v2 3/3] selftests/binderfs: add test for " Carlos Llamas
@ 2021-07-15  8:07   ` Christian Brauner
  2 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-07-15  8:07 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Thu, Jul 15, 2021 at 03:18:03AM +0000, Carlos Llamas wrote:
> Provide userspace with a mechanism to discover features supported by
> the binder driver to refrain from using any unsupported ones in the
> first place. Starting with "oneway_spam_detection" only new features
> are to be listed under binderfs and all previous ones are assumed to
> be supported.
> 
> Assuming an instance of binderfs has been mounted at /dev/binderfs,
> binder feature files can be found under /dev/binderfs/features/.
> Usage example:
> 
>   $ mkdir /dev/binderfs
>   $ mount -t binder binder /dev/binderfs
>   $ cat /dev/binderfs/features/oneway_spam_detection
>   1
> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---

Looks good,
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] docs: binderfs: add section about feature files
  2021-07-15  3:18   ` [PATCH v2 2/3] docs: binderfs: add section about " Carlos Llamas
@ 2021-07-15  8:08     ` Christian Brauner
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-07-15  8:08 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Thu, Jul 15, 2021 at 03:18:04AM +0000, Carlos Llamas wrote:
> Document how binder feature files can be used to determine whether a
> feature is supported by the binder driver. "oneway_spam_detection" is
> used as an example as it is the first available feature file.
> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---

Thanks,
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] selftests/binderfs: add test for feature files
  2021-07-15  3:18   ` [PATCH v2 3/3] selftests/binderfs: add test for " Carlos Llamas
@ 2021-07-15  8:09     ` Christian Brauner
  0 siblings, 0 replies; 12+ messages in thread
From: Christian Brauner @ 2021-07-15  8:09 UTC (permalink / raw)
  To: Carlos Llamas
  Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos,
	Martijn Coenen, Christian Brauner, Joel Fernandes,
	Steven Moreland, kernel-team, linux-kernel

On Thu, Jul 15, 2021 at 03:18:05AM +0000, Carlos Llamas wrote:
> Verify that feature files are created successfully after mounting a
> binderfs instance. Note that only "oneway_spam_detection" feature is
> tested with this patch as it is currently the only feature listed.
> 
> Signed-off-by: Carlos Llamas <cmllamas@google.com>
> ---

Looks good,
Acked-by: Christian Brauner <christian.brauner@ubuntu.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-07-15  8:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07 16:24 [PATCH] ANDROID: binderfs: add capabilities support Carlos Llamas
2021-07-07 16:59 ` Greg Kroah-Hartman
2021-07-08 16:33   ` Carlos Llamas
2021-07-09  8:39   ` Christian Brauner
2021-07-09  8:55 ` Christian Brauner
2021-07-09 16:32   ` Carlos Llamas
2021-07-15  3:18 ` [PATCH v2 1/3] binderfs: add support for feature files Carlos Llamas
2021-07-15  3:18   ` [PATCH v2 2/3] docs: binderfs: add section about " Carlos Llamas
2021-07-15  8:08     ` Christian Brauner
2021-07-15  3:18   ` [PATCH v2 3/3] selftests/binderfs: add test for " Carlos Llamas
2021-07-15  8:09     ` Christian Brauner
2021-07-15  8:07   ` [PATCH v2 1/3] binderfs: add support " Christian Brauner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.