Suggested-by: Alexey Dobriyan <[email protected]>
Reviewed-by: Alexey Dobriyan <[email protected]>
Signed-off-by: Alexey Gladkov <[email protected]>
---
include/linux/proc_fs.h | 9 +--------
include/uapi/linux/proc_fs.h | 13 +++++++++++++
2 files changed, 14 insertions(+), 8 deletions(-)
create mode 100644 include/uapi/linux/proc_fs.h
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index afd38cae2339..d259817ec913 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -7,6 +7,7 @@
#include <linux/types.h>
#include <linux/fs.h>
+#include <uapi/linux/proc_fs.h>
struct proc_dir_entry;
struct seq_file;
@@ -27,14 +28,6 @@ struct proc_ops {
unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
};
-/* definitions for hide_pid field */
-enum {
- HIDEPID_OFF = 0,
- HIDEPID_NO_ACCESS = 1,
- HIDEPID_INVISIBLE = 2,
- HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */
-};
-
/* definitions for proc mount option pidonly */
enum {
PROC_PIDONLY_OFF = 0,
diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
new file mode 100644
index 000000000000..dc6d717aa6ec
--- /dev/null
+++ b/include/uapi/linux/proc_fs.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_PROC_FS_H
+#define _UAPI_PROC_FS_H
+
+/* definitions for hide_pid field */
+enum {
+ HIDEPID_OFF = 0,
+ HIDEPID_NO_ACCESS = 1,
+ HIDEPID_INVISIBLE = 2,
+ HIDEPID_NOT_PTRACEABLE = 4,
+};
+
+#endif
--
2.25.2
On Fri, Mar 27, 2020 at 06:23:29PM +0100, Alexey Gladkov wrote:
> Suggested-by: Alexey Dobriyan <[email protected]>
> Reviewed-by: Alexey Dobriyan <[email protected]>
> Signed-off-by: Alexey Gladkov <[email protected]>
> ---
> include/linux/proc_fs.h | 9 +--------
> include/uapi/linux/proc_fs.h | 13 +++++++++++++
> 2 files changed, 14 insertions(+), 8 deletions(-)
> create mode 100644 include/uapi/linux/proc_fs.h
>
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index afd38cae2339..d259817ec913 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -7,6 +7,7 @@
>
> #include <linux/types.h>
> #include <linux/fs.h>
> +#include <uapi/linux/proc_fs.h>
>
> struct proc_dir_entry;
> struct seq_file;
> @@ -27,14 +28,6 @@ struct proc_ops {
> unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> };
>
> -/* definitions for hide_pid field */
> -enum {
> - HIDEPID_OFF = 0,
> - HIDEPID_NO_ACCESS = 1,
> - HIDEPID_INVISIBLE = 2,
> - HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */
> -};
> -
> /* definitions for proc mount option pidonly */
> enum {
> PROC_PIDONLY_OFF = 0,
> diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
> new file mode 100644
> index 000000000000..dc6d717aa6ec
> --- /dev/null
> +++ b/include/uapi/linux/proc_fs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_PROC_FS_H
> +#define _UAPI_PROC_FS_H
> +
> +/* definitions for hide_pid field */
> +enum {
> + HIDEPID_OFF = 0,
> + HIDEPID_NO_ACCESS = 1,
> + HIDEPID_INVISIBLE = 2,
> + HIDEPID_NOT_PTRACEABLE = 4,
> +};
> +
> +#endif
> --
> 2.25.2
>
Should the numeric values still be UAPI if there is string parsing now?
--
Kees Cook
On Sat, Mar 28, 2020 at 01:41:02PM -0700, Kees Cook wrote:
> diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
> > new file mode 100644
> > index 000000000000..dc6d717aa6ec
> > --- /dev/null
> > +++ b/include/uapi/linux/proc_fs.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef _UAPI_PROC_FS_H
> > +#define _UAPI_PROC_FS_H
> > +
> > +/* definitions for hide_pid field */
> > +enum {
> > + HIDEPID_OFF = 0,
> > + HIDEPID_NO_ACCESS = 1,
> > + HIDEPID_INVISIBLE = 2,
> > + HIDEPID_NOT_PTRACEABLE = 4,
> > +};
> > +
> > +#endif
> > --
> > 2.25.2
> >
>
> Should the numeric values still be UAPI if there is string parsing now?
I think yes, because these are still valid hidepid= values.
--
Rgrds, legion
On Sat, Mar 28, 2020 at 10:25:47PM +0100, Alexey Gladkov wrote:
> On Sat, Mar 28, 2020 at 01:41:02PM -0700, Kees Cook wrote:
> > diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
> > > new file mode 100644
> > > index 000000000000..dc6d717aa6ec
> > > --- /dev/null
> > > +++ b/include/uapi/linux/proc_fs.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI_PROC_FS_H
> > > +#define _UAPI_PROC_FS_H
> > > +
> > > +/* definitions for hide_pid field */
> > > +enum {
> > > + HIDEPID_OFF = 0,
> > > + HIDEPID_NO_ACCESS = 1,
> > > + HIDEPID_INVISIBLE = 2,
> > > + HIDEPID_NOT_PTRACEABLE = 4,
> > > +};
> > > +
> > > +#endif
> > > --
> > > 2.25.2
> > >
> >
> > Should the numeric values still be UAPI if there is string parsing now?
>
> I think yes, because these are still valid hidepid= values.
But if we don't expose the values, we can do whatever we like with
future numbers (e.g. the "is this a value or a bit field?" question).
--
Kees Cook
On Sat, Mar 28, 2020 at 02:53:49PM -0700, Kees Cook wrote:
> > > > +/* definitions for hide_pid field */
> > > > +enum {
> > > > + HIDEPID_OFF = 0,
> > > > + HIDEPID_NO_ACCESS = 1,
> > > > + HIDEPID_INVISIBLE = 2,
> > > > + HIDEPID_NOT_PTRACEABLE = 4,
> > > > +};
> > > Should the numeric values still be UAPI if there is string parsing now?
> >
> > I think yes, because these are still valid hidepid= values.
>
> But if we don't expose the values, we can do whatever we like with
> future numbers (e.g. the "is this a value or a bit field?" question).
Alexey Dobriyan suggested to put these parameters into the UAPI and it
makes sense because these are user parameters.
--
Rgrds, legion
On Sun, Mar 29, 2020 at 12:00:46AM +0100, Alexey Gladkov wrote:
> On Sat, Mar 28, 2020 at 02:53:49PM -0700, Kees Cook wrote:
> > > > > +/* definitions for hide_pid field */
> > > > > +enum {
> > > > > + HIDEPID_OFF = 0,
> > > > > + HIDEPID_NO_ACCESS = 1,
> > > > > + HIDEPID_INVISIBLE = 2,
> > > > > + HIDEPID_NOT_PTRACEABLE = 4,
> > > > > +};
> > > > Should the numeric values still be UAPI if there is string parsing now?
> > >
> > > I think yes, because these are still valid hidepid= values.
> >
> > But if we don't expose the values, we can do whatever we like with
> > future numbers (e.g. the "is this a value or a bit field?" question).
>
> Alexey Dobriyan suggested to put these parameters into the UAPI and it
> makes sense because these are user parameters.
Okidokey. :) Anyway, ignore my HIDEPID_MAX idea then, since this could
become a bitfield. Just checking for individual bits is the way to go
for now. Sorry for the noise.
--
Kees Cook
I will just say that I do not understand exporting this to the uapi
headers. Why do we want to export the enumeration names?
I understand that the values are uapi. This looks like it will make it
difficult to make changes that rename enumeration values to make
the code more readable.
Given that this patchset goes immediately to using string enumerated
values, I also don't understand the point of exporting
HIDEPID_NOT_PTRACEABLE. I don't think we need to ever let
people use the numeric value.
My sense is that if we are switching to string values we should
just leave the existing numeric values as backwards compatiblity
and not do anything to make them easier to use.
Eric
Alexey Gladkov <[email protected]> writes:
> Suggested-by: Alexey Dobriyan <[email protected]>
> Reviewed-by: Alexey Dobriyan <[email protected]>
> Signed-off-by: Alexey Gladkov <[email protected]>
> ---
> include/linux/proc_fs.h | 9 +--------
> include/uapi/linux/proc_fs.h | 13 +++++++++++++
> 2 files changed, 14 insertions(+), 8 deletions(-)
> create mode 100644 include/uapi/linux/proc_fs.h
>
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index afd38cae2339..d259817ec913 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -7,6 +7,7 @@
>
> #include <linux/types.h>
> #include <linux/fs.h>
> +#include <uapi/linux/proc_fs.h>
>
> struct proc_dir_entry;
> struct seq_file;
> @@ -27,14 +28,6 @@ struct proc_ops {
> unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> };
>
> -/* definitions for hide_pid field */
> -enum {
> - HIDEPID_OFF = 0,
> - HIDEPID_NO_ACCESS = 1,
> - HIDEPID_INVISIBLE = 2,
> - HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */
> -};
> -
> /* definitions for proc mount option pidonly */
> enum {
> PROC_PIDONLY_OFF = 0,
> diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
> new file mode 100644
> index 000000000000..dc6d717aa6ec
> --- /dev/null
> +++ b/include/uapi/linux/proc_fs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI_PROC_FS_H
> +#define _UAPI_PROC_FS_H
> +
> +/* definitions for hide_pid field */
> +enum {
> + HIDEPID_OFF = 0,
> + HIDEPID_NO_ACCESS = 1,
> + HIDEPID_INVISIBLE = 2,
> + HIDEPID_NOT_PTRACEABLE = 4,
> +};
> +
> +#endif
On Thu, Apr 02, 2020 at 11:58:28AM -0500, Eric W. Biederman wrote:
>
> I will just say that I do not understand exporting this to the uapi
> headers. Why do we want to export the enumeration names?
>
> I understand that the values are uapi. This looks like it will make it
> difficult to make changes that rename enumeration values to make
> the code more readable.
>
> Given that this patchset goes immediately to using string enumerated
> values, I also don't understand the point of exporting
> HIDEPID_NOT_PTRACEABLE. I don't think we need to ever let
> people use the numeric value.
>
> My sense is that if we are switching to string values we should
> just leave the existing numeric values as backwards compatiblity
> and not do anything to make them easier to use.
Yeah, that's what I had suggested too. Let's not export this to UAPI.
-Kees
>
> Eric
>
>
> Alexey Gladkov <[email protected]> writes:
>
> > Suggested-by: Alexey Dobriyan <[email protected]>
> > Reviewed-by: Alexey Dobriyan <[email protected]>
> > Signed-off-by: Alexey Gladkov <[email protected]>
> > ---
> > include/linux/proc_fs.h | 9 +--------
> > include/uapi/linux/proc_fs.h | 13 +++++++++++++
> > 2 files changed, 14 insertions(+), 8 deletions(-)
> > create mode 100644 include/uapi/linux/proc_fs.h
> >
> > diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> > index afd38cae2339..d259817ec913 100644
> > --- a/include/linux/proc_fs.h
> > +++ b/include/linux/proc_fs.h
> > @@ -7,6 +7,7 @@
> >
> > #include <linux/types.h>
> > #include <linux/fs.h>
> > +#include <uapi/linux/proc_fs.h>
> >
> > struct proc_dir_entry;
> > struct seq_file;
> > @@ -27,14 +28,6 @@ struct proc_ops {
> > unsigned long (*proc_get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
> > };
> >
> > -/* definitions for hide_pid field */
> > -enum {
> > - HIDEPID_OFF = 0,
> > - HIDEPID_NO_ACCESS = 1,
> > - HIDEPID_INVISIBLE = 2,
> > - HIDEPID_NOT_PTRACEABLE = 4, /* Limit pids to only ptraceable pids */
> > -};
> > -
> > /* definitions for proc mount option pidonly */
> > enum {
> > PROC_PIDONLY_OFF = 0,
> > diff --git a/include/uapi/linux/proc_fs.h b/include/uapi/linux/proc_fs.h
> > new file mode 100644
> > index 000000000000..dc6d717aa6ec
> > --- /dev/null
> > +++ b/include/uapi/linux/proc_fs.h
> > @@ -0,0 +1,13 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef _UAPI_PROC_FS_H
> > +#define _UAPI_PROC_FS_H
> > +
> > +/* definitions for hide_pid field */
> > +enum {
> > + HIDEPID_OFF = 0,
> > + HIDEPID_NO_ACCESS = 1,
> > + HIDEPID_INVISIBLE = 2,
> > + HIDEPID_NOT_PTRACEABLE = 4,
> > +};
> > +
> > +#endif
--
Kees Cook