[PATCH v2 0/5] ipc: Clamp *mni to the real IPCMNI limit

[PATCH v2 0/5] ipc: Clamp *mni to the real IPCMNI limit

[Waiman Long]

v1->v2:
 - Add kdoc comments to the do_proc_do{u}intvec_minmax_conv_param
   structures.
 - Add a new flags field to the ctl_table structure for specifying
   whether range clamping should be activated instead of adding new
   sysctl parameter handlers.
 - Clamp the semmni value embedded in the multi-values sem parameter.

v1 patch: https://lkml.org/lkml/2018/2/19/453

The sysctl parameters msgmni, shmmni and semmni have an inherent limit
of IPC_MNI (32k). However, users may not be aware of that because they
can write a value much higher than that without getting any error or
notification. Reading the parameters back will show the newly written
values which are not real.

Enforcing the limit by failing sysctl parameter write, however, can
break existing user applications. To address this delemma, a new flags
field is introduced into the ctl_table. The value CTL_FLAGS_CLAMP_RANGE
can be added to any ctl_table entries to enable a looser range clamping
without returning any error. For example,

  .flags = CTL_FLAGS_CLAMP_RANGE,

This flags value are now used for the range checking of shmmni,
msgmni and semmni without breaking existing applications. If any out
of range value is written to those sysctl parameters, the following
warning will be printed instead.

  Kernel parameter "shmmni" was set out of range [0, 32768], clamped to 32768.

Reading the values back will show 32768 instead of some fake values.

Waiman Long (5):
  sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param
  sysctl: Add flags to support min/max range clamping
  sysctl: Warn when a clamped sysctl parameter is set out of range
  ipc: Clamp msgmni and shmmni to the real IPCMNI limit
  ipc: Clamp semmni to the real IPCMNI limit

 include/linux/sysctl.h |   7 ++++
 ipc/ipc_sysctl.c       |  29 +++++++++++--
 ipc/sem.c              |  33 +++++++++++++++
 ipc/util.h             |   4 ++
 kernel/sysctl.c        | 111 +++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 173 insertions(+), 11 deletions(-)

-- 
1.8.3.1

[PATCH v2 1/5] sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param

[Waiman Long]

Kdoc comments are added to the do_proc_dointvec_minmax_conv_param
and do_proc_douintvec_minmax_conv_param structures thare are used
internally for range checking.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/sysctl.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f98f28c..52b647a 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2500,6 +2500,17 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
 }
 #endif
 
+/**
+ * DOC: do_proc_dointvec_minmax_conv_param
+ *
+ * The do_proc_dointvec_minmax_conv_param structure provides the
+ * minimum and maximum values for doing range checking for those sysctl
+ * parameters that use the proc_dointvec_minmax() handler. The error
+ * code -EINVAL will be returned if the range check fails.
+ *
+ *  min: ptr to minimum allowable value
+ *  max: ptr to maximum allowable value
+ */
 struct do_proc_dointvec_minmax_conv_param {
 	int *min;
 	int *max;
@@ -2556,6 +2567,17 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
 				do_proc_dointvec_minmax_conv, &param);
 }
 
+/**
+ * DOC: do_proc_douintvec_minmax_conv_param
+ *
+ * The do_proc_dointvec_minmax_conv_param structure provides the
+ * minimum and maximum values for doing range checking for those sysctl
+ * parameters that use the proc_douintvec_minmax() handler. The error
+ * code -ERANGE will be returned if the range check fails.
+ *
+ *  min: ptr to minimum allowable value
+ *  max: ptr to maximum allowable value
+ */
 struct do_proc_douintvec_minmax_conv_param {
 	unsigned int *min;
 	unsigned int *max;
-- 
1.8.3.1

[PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Waiman Long]

When minimum/maximum values are specified for a sysctl parameter in
the ctl_table structure with proc_dointvec_minmax() handler, update
to that parameter will fail with error if the given value is outside
of the required range.

There are use cases where it may be better to clamp the value of
the sysctl parameter to the given range without failing the update,
especially if the users are not aware of the actual range limits.
Reading the value back after the update will now be a good practice
to see if the provided value exceeds the range limits.

To provide this less restrictive form of range checking, a new flags
field is added to the ctl_table structure. The new field is a 16-bit
value that just fits into the hole left by the 16-bit umode_t field
without increasing the size of the structure.

When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table entry,
any update from the userspace will be clamped to the given range
without error.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/sysctl.h |  6 ++++++
 kernel/sysctl.c        | 58 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index b769ecf..eceeaee 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -116,6 +116,7 @@ struct ctl_table
 	void *data;
 	int maxlen;
 	umode_t mode;
+	uint16_t flags;
 	struct ctl_table *child;	/* Deprecated */
 	proc_handler *proc_handler;	/* Callback for text formatting */
 	struct ctl_table_poll *poll;
@@ -123,6 +124,11 @@ struct ctl_table
 	void *extra2;
 } __randomize_layout;
 
+/*
+ * ctl_table flags (16 different flags, at most)
+ */
+#define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */
+
 struct ctl_node {
 	struct rb_node node;
 	struct ctl_table_header *header;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 52b647a..2b2b30c 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2505,15 +2505,21 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
  *
  * The do_proc_dointvec_minmax_conv_param structure provides the
  * minimum and maximum values for doing range checking for those sysctl
- * parameters that use the proc_dointvec_minmax() handler. The error
- * code -EINVAL will be returned if the range check fails.
+ * parameters that use the proc_dointvec_minmax() handler.
+ *
+ * The error code -EINVAL will be returned if the range check fails
+ * and the CTL_FLAGS_CLAMP_RANGE bit is not set in the given flags.
+ * If that flag is set, the new sysctl value will be clamped to the
+ * given range without returning any error.
  *
  *  min: ptr to minimum allowable value
  *  max: ptr to maximum allowable value
+ *  flags: ptr to flags
  */
 struct do_proc_dointvec_minmax_conv_param {
 	int *min;
 	int *max;
+	uint16_t *flags;
 };
 
 static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
@@ -2523,9 +2529,21 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 	struct do_proc_dointvec_minmax_conv_param *param = data;
 	if (write) {
 		int val = *negp ? -*lvalp : *lvalp;
-		if ((param->min && *param->min > val) ||
-		    (param->max && *param->max < val))
-			return -EINVAL;
+		bool clamp = param->flags &&
+			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
+
+		if (param->min && *param->min > val) {
+			if (clamp)
+				val = *param->min;
+			else
+				return -EINVAL;
+		}
+		if (param->max && *param->max < val) {
+			if (clamp)
+				val = *param->max;
+			else
+				return -EINVAL;
+		}
 		*valp = val;
 	} else {
 		int val = *valp;
@@ -2562,6 +2580,7 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
 	struct do_proc_dointvec_minmax_conv_param param = {
 		.min = (int *) table->extra1,
 		.max = (int *) table->extra2,
+		.flags = &table->flags,
 	};
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 				do_proc_dointvec_minmax_conv, &param);
@@ -2572,15 +2591,21 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
  *
  * The do_proc_dointvec_minmax_conv_param structure provides the
  * minimum and maximum values for doing range checking for those sysctl
- * parameters that use the proc_douintvec_minmax() handler. The error
- * code -ERANGE will be returned if the range check fails.
+ * parameters that use the proc_douintvec_minmax() handler.
+ *
+ * The error code -ERANGE will be returned if the range check fails
+ * and the CTL_FLAGS_CLAMP_RANGE bit is not set in the given flags.
+ * If that flag is set, the new sysctl value will be clamped to the
+ * given range without returning any error.
  *
  *  min: ptr to minimum allowable value
  *  max: ptr to maximum allowable value
+ *  flags: ptr to flags
  */
 struct do_proc_douintvec_minmax_conv_param {
 	unsigned int *min;
 	unsigned int *max;
+	uint16_t *flags;
 };
 
 static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
@@ -2591,14 +2616,24 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 
 	if (write) {
 		unsigned int val = *lvalp;
+		bool clamp = param->flags &&
+			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
 
 		if (*lvalp > UINT_MAX)
 			return -EINVAL;
 
-		if ((param->min && *param->min > val) ||
-		    (param->max && *param->max < val))
-			return -ERANGE;
-
+		if (param->min && *param->min > val) {
+			if (clamp)
+				val = *param->min;
+			else
+				return -ERANGE;
+		}
+		if (param->max && *param->max < val) {
+			if (clamp)
+				val = *param->max;
+			else
+				return -ERANGE;
+		}
 		*valp = val;
 	} else {
 		unsigned int val = *valp;
@@ -2633,6 +2668,7 @@ int proc_douintvec_minmax(struct ctl_table *table, int write,
 	struct do_proc_douintvec_minmax_conv_param param = {
 		.min = (unsigned int *) table->extra1,
 		.max = (unsigned int *) table->extra2,
+		.flags = &table->flags,
 	};
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
-- 
1.8.3.1

[PATCH v2 3/5] sysctl: Warn when a clamped sysctl parameter is set out of range

[Waiman Long]

Even with clamped sysctl parameters, it is still not that straight
forward to figure out the exact range of those parameters. One may
try to write extreme parameter values to see if they get clamped.
To make it easier, a warning with the expected range will now be
printed in the kernel ring buffer when a clamped sysctl parameter
receives an out of range value.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/sysctl.h |  1 +
 kernel/sysctl.c        | 55 ++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index eceeaee..4e4f74a2 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -128,6 +128,7 @@ struct ctl_table
  * ctl_table flags (16 different flags, at most)
  */
 #define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */
+#define CTL_FLAGS_OOR_WARNED	(1 << 1) /* Out-of-range warning issued */
 
 struct ctl_node {
 	struct rb_node node;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 2b2b30c..f9f3373 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2515,36 +2515,54 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
  *  min: ptr to minimum allowable value
  *  max: ptr to maximum allowable value
  *  flags: ptr to flags
+ *  name: sysctl parameter name
  */
 struct do_proc_dointvec_minmax_conv_param {
 	int *min;
 	int *max;
 	uint16_t *flags;
+	const char *name;
 };
 
 static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 					int *valp,
 					int write, void *data)
 {
+#define SYSCTL_WARN_MSG	\
+"Kernel parameter \"%s\" was set out of range [%d, %d], clamped to %d.\n"
+
 	struct do_proc_dointvec_minmax_conv_param *param = data;
+
 	if (write) {
 		int val = *negp ? -*lvalp : *lvalp;
+		bool clamped = false;
 		bool clamp = param->flags &&
 			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
 
 		if (param->min && *param->min > val) {
-			if (clamp)
+			if (clamp) {
 				val = *param->min;
-			else
+				clamped = true;
+			} else {
 				return -EINVAL;
+			}
 		}
 		if (param->max && *param->max < val) {
-			if (clamp)
+			if (clamp) {
 				val = *param->max;
-			else
+				clamped = true;
+			} else {
 				return -EINVAL;
+			}
 		}
 		*valp = val;
+		if (clamped && param->name &&
+		   !(*param->flags & CTL_FLAGS_OOR_WARNED)) {
+			pr_warn(SYSCTL_WARN_MSG, param->name,
+				param->min ? *param->min : -INT_MAX,
+				param->max ? *param->max :  INT_MAX, val);
+			*param->flags |= CTL_FLAGS_OOR_WARNED;
+		}
 	} else {
 		int val = *valp;
 		if (val < 0) {
@@ -2556,6 +2574,7 @@ static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
 		}
 	}
 	return 0;
+#undef SYSCTL_WARN_MSG
 }
 
 /**
@@ -2581,6 +2600,7 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
 		.min = (int *) table->extra1,
 		.max = (int *) table->extra2,
 		.flags = &table->flags,
+		.name  = table->procname,
 	};
 	return do_proc_dointvec(table, write, buffer, lenp, ppos,
 				do_proc_dointvec_minmax_conv, &param);
@@ -2601,21 +2621,27 @@ int proc_dointvec_minmax(struct ctl_table *table, int write,
  *  min: ptr to minimum allowable value
  *  max: ptr to maximum allowable value
  *  flags: ptr to flags
+ *  name: sysctl parameter name
  */
 struct do_proc_douintvec_minmax_conv_param {
 	unsigned int *min;
 	unsigned int *max;
 	uint16_t *flags;
+	const char *name;
 };
 
 static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 					 unsigned int *valp,
 					 int write, void *data)
 {
+#define SYSCTL_WARN_MSG	\
+"Kernel parameter \"%s\" was set out of range [%u, %u], clamped to %u.\n"
+
 	struct do_proc_douintvec_minmax_conv_param *param = data;
 
 	if (write) {
 		unsigned int val = *lvalp;
+		bool clamped = false;
 		bool clamp = param->flags &&
 			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
 
@@ -2623,24 +2649,36 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
 			return -EINVAL;
 
 		if (param->min && *param->min > val) {
-			if (clamp)
+			if (clamp) {
 				val = *param->min;
-			else
+				clamped = true;
+			} else {
 				return -ERANGE;
+			}
 		}
 		if (param->max && *param->max < val) {
-			if (clamp)
+			if (clamp) {
 				val = *param->max;
-			else
+				clamped = true;
+			} else {
 				return -ERANGE;
+			}
 		}
 		*valp = val;
+		if (clamped && param->name &&
+		   !(*param->flags & CTL_FLAGS_OOR_WARNED)) {
+			pr_warn(SYSCTL_WARN_MSG, param->name,
+				param->min ? *param->min : 0,
+				param->max ? *param->max : UINT_MAX, val);
+			*param->flags |= CTL_FLAGS_OOR_WARNED;
+		}
 	} else {
 		unsigned int val = *valp;
 		*lvalp = (unsigned long) val;
 	}
 
 	return 0;
+#undef SYSCTL_WARN_MSG
 }
 
 /**
@@ -2669,6 +2707,7 @@ int proc_douintvec_minmax(struct ctl_table *table, int write,
 		.min = (unsigned int *) table->extra1,
 		.max = (unsigned int *) table->extra2,
 		.flags = &table->flags,
+		.name  = table->procname,
 	};
 	return do_proc_douintvec(table, write, buffer, lenp, ppos,
 				 do_proc_douintvec_minmax_conv, &param);
-- 
1.8.3.1

[PATCH v2 4/5] ipc: Clamp msgmni and shmmni to the real IPCMNI limit

[Waiman Long]

A user can write arbitrary integer values to msgmni and shmmni sysctl
parameters without getting error, but the actual limit is really
IPCMNI (32k). This can mislead users as they think they can get a
value that is not real.

Enforcing the limit by failing the sysctl parameter write, however,
can break existing user applications. Instead, the range clamping flag
is set to enforce the limit without failing existing user code. Users
can easily figure out if the sysctl parameter value is out of range
by either reading back the parameter value or checking the kernel
ring buffer for warning.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 ipc/ipc_sysctl.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index 8ad93c2..e4ab272 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -41,12 +41,17 @@ static int proc_ipc_dointvec(struct ctl_table *table, int write,
 static int proc_ipc_dointvec_minmax(struct ctl_table *table, int write,
 	void __user *buffer, size_t *lenp, loff_t *ppos)
 {
+	int ret;
 	struct ctl_table ipc_table;
 
 	memcpy(&ipc_table, table, sizeof(ipc_table));
 	ipc_table.data = get_ipc(table);
 
-	return proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
+	ret = proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
+
+	table->flags |= ipc_table.flags; /* Copy back any change in flags */
+
+	return ret;
 }
 
 static int proc_ipc_dointvec_minmax_orphans(struct ctl_table *table, int write,
@@ -99,6 +104,7 @@ static int proc_ipc_auto_msgmni(struct ctl_table *table, int write,
 static int zero;
 static int one = 1;
 static int int_max = INT_MAX;
+static int ipc_mni = IPCMNI;
 
 static struct ctl_table ipc_kern_table[] = {
 	{
@@ -120,7 +126,10 @@ static int proc_ipc_auto_msgmni(struct ctl_table *table, int write,
 		.data		= &init_ipc_ns.shm_ctlmni,
 		.maxlen		= sizeof(init_ipc_ns.shm_ctlmni),
 		.mode		= 0644,
-		.proc_handler	= proc_ipc_dointvec,
+		.proc_handler	= proc_ipc_dointvec_minmax,
+		.extra1		= &zero,
+		.extra2		= &ipc_mni,
+		.flags		= CTL_FLAGS_CLAMP_RANGE,
 	},
 	{
 		.procname	= "shm_rmid_forced",
@@ -147,7 +156,8 @@ static int proc_ipc_auto_msgmni(struct ctl_table *table, int write,
 		.mode		= 0644,
 		.proc_handler	= proc_ipc_dointvec_minmax,
 		.extra1		= &zero,
-		.extra2		= &int_max,
+		.extra2		= &ipc_mni,
+		.flags		= CTL_FLAGS_CLAMP_RANGE,
 	},
 	{
 		.procname	= "auto_msgmni",
-- 
1.8.3.1

[PATCH v2 5/5] ipc: Clamp semmni to the real IPCMNI limit

[Waiman Long]

This patch clamps the semmni value (fourth element of sem_ctls[]
array) to within the [0, IPCMNI] range and prints a warning message
once when an out-of-range value is being written.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 ipc/ipc_sysctl.c | 13 ++++++++++++-
 ipc/sem.c        | 33 +++++++++++++++++++++++++++++++++
 ipc/util.h       |  4 ++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
index e4ab272..5a5b50b 100644
--- a/ipc/ipc_sysctl.c
+++ b/ipc/ipc_sysctl.c
@@ -93,12 +93,22 @@ static int proc_ipc_auto_msgmni(struct ctl_table *table, int write,
 	return proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
 }
 
+static int proc_ipc_sem_dointvec(struct ctl_table *table, int write,
+	void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	int ret = proc_ipc_dointvec(table, write, buffer, lenp, ppos);
+
+	sem_check_semmni(table, current->nsproxy->ipc_ns);
+	return ret;
+}
+
 #else
 #define proc_ipc_doulongvec_minmax NULL
 #define proc_ipc_dointvec	   NULL
 #define proc_ipc_dointvec_minmax   NULL
 #define proc_ipc_dointvec_minmax_orphans   NULL
 #define proc_ipc_auto_msgmni	   NULL
+#define proc_ipc_sem_dointvec	   NULL
 #endif
 
 static int zero;
@@ -182,7 +192,8 @@ static int proc_ipc_auto_msgmni(struct ctl_table *table, int write,
 		.data		= &init_ipc_ns.sem_ctls,
 		.maxlen		= 4*sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_ipc_dointvec,
+		.proc_handler	= proc_ipc_sem_dointvec,
+		.flags		= CTL_FLAGS_CLAMP_RANGE,
 	},
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	{
diff --git a/ipc/sem.c b/ipc/sem.c
index a4af049..0af13e8 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -2337,3 +2337,36 @@ static int sysvipc_sem_proc_show(struct seq_file *s, void *it)
 	return 0;
 }
 #endif
+
+#ifdef CONFIG_PROC_SYSCTL
+/*
+ * Check to see if semmni is out of range and clamp it if necessary.
+ */
+void sem_check_semmni(struct ctl_table *table, struct ipc_namespace *ns)
+{
+#define SYSCTL_WARN_MSG	\
+"Kernel parameter \"sem[3]\" was set out of range [%d, %d], clamped to %d.\n"
+
+	bool clamped = false;
+
+	if (!(table->flags & CTL_FLAGS_CLAMP_RANGE))
+		return;
+
+	/*
+	 * Clamp semmni to the range [0, IPCMNI].
+	 */
+	if (ns->sc_semmni < 0) {
+		ns->sc_semmni = 0;
+		clamped = true;
+	}
+	if (ns->sc_semmni > IPCMNI) {
+		ns->sc_semmni = IPCMNI;
+		clamped = true;
+	}
+	if (clamped && !(table->flags & CTL_FLAGS_OOR_WARNED)) {
+		pr_warn(SYSCTL_WARN_MSG, 0, IPCMNI, ns->sc_semmni);
+		table->flags |= CTL_FLAGS_OOR_WARNED;
+	}
+#undef SYSCTL_WARN_MSG
+}
+#endif
diff --git a/ipc/util.h b/ipc/util.h
index 89b8ec1..af57394 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -206,6 +206,10 @@ int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
 void free_ipcs(struct ipc_namespace *ns, struct ipc_ids *ids,
 		void (*free)(struct ipc_namespace *, struct kern_ipc_perm *));
 
+#ifdef CONFIG_PROC_SYSCTL
+extern void sem_check_semmni(struct ctl_table *table, struct ipc_namespace *ns);
+#endif
+
 #ifdef CONFIG_COMPAT
 #include <linux/compat.h>
 struct compat_ipc_perm {
-- 
1.8.3.1

Re: [PATCH v2 1/5] sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param

[Matthew Wilcox]

On Tue, Feb 27, 2018 at 03:49:47PM -0500, Waiman Long wrote:
> +/**
> + * DOC: do_proc_dointvec_minmax_conv_param
> + *
> + * The do_proc_dointvec_minmax_conv_param structure provides the
> + * minimum and maximum values for doing range checking for those sysctl
> + * parameters that use the proc_dointvec_minmax() handler. The error
> + * code -EINVAL will be returned if the range check fails.
> + *
> + *  min: ptr to minimum allowable value
> + *  max: ptr to maximum allowable value
> + */

This isn't how to document a struct; see Documentation/doc-guide/kernel-doc.rst

/**
 * struct do_proc_dointvec_minmax_conv_param - Range checking for sysctls
 * @min: Pointer to minimum allowable value.
 * @max: Pointer to maximum allowable value.
 *
 * Provides the minimum and maximum values allowed for a parameter.  A
 * pointer to this structure should be passed to proc_dointvec_minmax().
 */

Also, it's inappropriate to document the return value from
proc_dointvec_minmax() here; that should be in the kernel-doc for the
function.

Re: [PATCH v2 1/5] sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param

[Waiman Long]

On 02/27/2018 04:10 PM, Matthew Wilcox wrote:
> On Tue, Feb 27, 2018 at 03:49:47PM -0500, Waiman Long wrote:
>> +/**
>> + * DOC: do_proc_dointvec_minmax_conv_param
>> + *
>> + * The do_proc_dointvec_minmax_conv_param structure provides the
>> + * minimum and maximum values for doing range checking for those sysctl
>> + * parameters that use the proc_dointvec_minmax() handler. The error
>> + * code -EINVAL will be returned if the range check fails.
>> + *
>> + *  min: ptr to minimum allowable value
>> + *  max: ptr to maximum allowable value
>> + */
> This isn't how to document a struct; see Documentation/doc-guide/kernel-doc.rst
>
> /**
>  * struct do_proc_dointvec_minmax_conv_param - Range checking for sysctls
>  * @min: Pointer to minimum allowable value.
>  * @max: Pointer to maximum allowable value.
>  *
>  * Provides the minimum and maximum values allowed for a parameter.  A
>  * pointer to this structure should be passed to proc_dointvec_minmax().
>  */
>
> Also, it's inappropriate to document the return value from
> proc_dointvec_minmax() here; that should be in the kernel-doc for the
> function.
>
Thanks for the advice. I will change the comments in the v3 patch.

-Longman

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Luis R. Rodriguez]

On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
> When minimum/maximum values are specified for a sysctl parameter in
> the ctl_table structure with proc_dointvec_minmax() handler,

an

> update
> to that parameter will fail with error if the given value is outside
> of the required range.
> 
> There are use cases where it may be better to clamp the value of
> the sysctl parameter to the given range without failing the update,
> especially if the users are not aware of the actual range limits.

Makes me wonder if we should add something which does let one query
for the ranges. Then scripts can fetch that as well.

> Reading the value back after the update will now be a good practice
> to see if the provided value exceeds the range limits.
> 
> To provide this less restrictive form of range checking, a new flags
> field is added to the ctl_table structure. The new field is a 16-bit
> value that just fits into the hole left by the 16-bit umode_t field
> without increasing the size of the structure.
> 
> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table entry,
> any update from the userspace will be clamped to the given range
> without error.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  include/linux/sysctl.h |  6 ++++++
>  kernel/sysctl.c        | 58 ++++++++++++++++++++++++++++++++++++++++----------
>  2 files changed, 53 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index b769ecf..eceeaee 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -116,6 +116,7 @@ struct ctl_table
>  	void *data;
>  	int maxlen;
>  	umode_t mode;
> +	uint16_t flags;
>  	struct ctl_table *child;	/* Deprecated */
>  	proc_handler *proc_handler;	/* Callback for text formatting */
>  	struct ctl_table_poll *poll;
> @@ -123,6 +124,11 @@ struct ctl_table
>  	void *extra2;
>  } __randomize_layout;
>  
> +/*
> + * ctl_table flags (16 different flags, at most)
> + */
> +#define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */

Since its only 16 best we kdocify, we can do so with

/**                                                                             
 * enum ctl_table_flags - flags for the ctl table
 *
 * @CTL_FLAGS_CLAMP_RANGE: If set this indicates that the entry should be
 *	flexibly clamp to min/max range in case the user provided an incorrect
 *	value.
 */
enum ctl_table_flags {
	CTL_FLAGS_CLAMP_RANGE		= BIT(0),
}

This lets us document this nicely.

> +
>  struct ctl_node {
>  	struct rb_node node;
>  	struct ctl_table_header *header;
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 52b647a..2b2b30c 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2505,15 +2505,21 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>   *
>   * The do_proc_dointvec_minmax_conv_param structure provides the
>   * minimum and maximum values for doing range checking for those sysctl
> - * parameters that use the proc_dointvec_minmax() handler. The error
> - * code -EINVAL will be returned if the range check fails.
> + * parameters that use the proc_dointvec_minmax() handler.
> + *
> + * The error code -EINVAL will be returned if the range check fails
> + * and the CTL_FLAGS_CLAMP_RANGE bit is not set in the given flags.
> + * If that flag is set, the new sysctl value will be clamped to the
> + * given range without returning any error.

This last part seems odd, we silently set the value to a limit if the
user set an invalid value?

Since this is actually not really undefined documenting that we set it
to the max value if the input value is greater than the max allowed would
be good. Likewise for the minimum.

  Luis

Re: [PATCH v2 3/5] sysctl: Warn when a clamped sysctl parameter is set out of range

[Luis R. Rodriguez]

On Tue, Feb 27, 2018 at 03:49:49PM -0500, Waiman Long wrote:
> Even with clamped sysctl parameters, it is still not that straight
> forward to figure out the exact range of those parameters. One may
> try to write extreme parameter values to see if they get clamped.
> To make it easier, a warning with the expected range will now be
> printed in the kernel ring buffer when a clamped sysctl parameter
> receives an out of range value.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  include/linux/sysctl.h |  1 +
>  kernel/sysctl.c        | 55 ++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index eceeaee..4e4f74a2 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -128,6 +128,7 @@ struct ctl_table
>   * ctl_table flags (16 different flags, at most)
>   */
>  #define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */
> +#define CTL_FLAGS_OOR_WARNED	(1 << 1) /* Out-of-range warning issued */

With the enum set you can then kdocify this too.

>  struct ctl_node {
>  	struct rb_node node;
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 2b2b30c..f9f3373 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -2515,36 +2515,54 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>   *  min: ptr to minimum allowable value
>   *  max: ptr to maximum allowable value
>   *  flags: ptr to flags
> + *  name: sysctl parameter name
>   */
>  struct do_proc_dointvec_minmax_conv_param {
>  	int *min;
>  	int *max;
>  	uint16_t *flags;
> +	const char *name;
>  };
>  
>  static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
>  					int *valp,
>  					int write, void *data)
>  {
> +#define SYSCTL_WARN_MSG	\
> +"Kernel parameter \"%s\" was set out of range [%d, %d], clamped to %d.\n"

Please loose this define. What about a proc_ctl_warn() which takes the
parameters and then does the checks?

> +
>  	struct do_proc_douintvec_minmax_conv_param *param = data;
>  
>  	if (write) {
>  		unsigned int val = *lvalp;
> +		bool clamped = false;
>  		bool clamp = param->flags &&
>  			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
>  
> @@ -2623,24 +2649,36 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
>  			return -EINVAL;
>  
>  		if (param->min && *param->min > val) {
> -			if (clamp)
> +			if (clamp) {
>  				val = *param->min;
> -			else
> +				clamped = true;
> +			} else {
>  				return -ERANGE;
> +			}
>  		}
>  		if (param->max && *param->max < val) {
> -			if (clamp)
> +			if (clamp) {
>  				val = *param->max;
> -			else
> +				clamped = true;
> +			} else {
>  				return -ERANGE;
> +			}
>  		}
>  		*valp = val;
> +		if (clamped && param->name &&
> +		   !(*param->flags & CTL_FLAGS_OOR_WARNED)) {
> +			pr_warn(SYSCTL_WARN_MSG, param->name,
> +				param->min ? *param->min : 0,
> +				param->max ? *param->max : UINT_MAX, val);
> +			*param->flags |= CTL_FLAGS_OOR_WARNED;

Why not just use pr_warn_once()?

  Luis

Re: [PATCH v2 4/5] ipc: Clamp msgmni and shmmni to the real IPCMNI limit

[Luis R. Rodriguez]

On Tue, Feb 27, 2018 at 03:49:50PM -0500, Waiman Long wrote:
> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> index 8ad93c2..e4ab272 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -41,12 +41,17 @@ static int proc_ipc_dointvec(struct ctl_table *table, int write,
>  static int proc_ipc_dointvec_minmax(struct ctl_table *table, int write,
>  	void __user *buffer, size_t *lenp, loff_t *ppos)
>  {
> +	int ret;
>  	struct ctl_table ipc_table;
>  
>  	memcpy(&ipc_table, table, sizeof(ipc_table));
>  	ipc_table.data = get_ipc(table);
>  
> -	return proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
> +	ret = proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
> +
> +	table->flags |= ipc_table.flags; /* Copy back any change in flags */

This seems fragile, why are we requiring this to be done by the users of
CTL_FLAGS_CLAMP_RANGE ?

  Luis

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Waiman Long]

On 02/27/2018 07:47 PM, Luis R. Rodriguez wrote:
> On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
>> When minimum/maximum values are specified for a sysctl parameter in
>> the ctl_table structure with proc_dointvec_minmax() handler,
> an
>
>> update
>> to that parameter will fail with error if the given value is outside
>> of the required range.
>>
>> There are use cases where it may be better to clamp the value of
>> the sysctl parameter to the given range without failing the update,
>> especially if the users are not aware of the actual range limits.
> Makes me wonder if we should add something which does let one query
> for the ranges. Then scripts can fetch that as well.

That will actually be better than printing out the range in the dmesg
log. However, I haven't figured out an easy way of doing that. If you
have any suggestion, please let me know about it.

>
>> Reading the value back after the update will now be a good practice
>> to see if the provided value exceeds the range limits.
>>
>> To provide this less restrictive form of range checking, a new flags
>> field is added to the ctl_table structure. The new field is a 16-bit
>> value that just fits into the hole left by the 16-bit umode_t field
>> without increasing the size of the structure.
>>
>> When the CTL_FLAGS_CLAMP_RANGE flag is set in the ctl_table entry,
>> any update from the userspace will be clamped to the given range
>> without error.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>  include/linux/sysctl.h |  6 ++++++
>>  kernel/sysctl.c        | 58 ++++++++++++++++++++++++++++++++++++++++----------
>>  2 files changed, 53 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index b769ecf..eceeaee 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -116,6 +116,7 @@ struct ctl_table
>>  	void *data;
>>  	int maxlen;
>>  	umode_t mode;
>> +	uint16_t flags;
>>  	struct ctl_table *child;	/* Deprecated */
>>  	proc_handler *proc_handler;	/* Callback for text formatting */
>>  	struct ctl_table_poll *poll;
>> @@ -123,6 +124,11 @@ struct ctl_table
>>  	void *extra2;
>>  } __randomize_layout;
>>  
>> +/*
>> + * ctl_table flags (16 different flags, at most)
>> + */
>> +#define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */
> Since its only 16 best we kdocify, we can do so with
>
> /**                                                                             
>  * enum ctl_table_flags - flags for the ctl table
>  *
>  * @CTL_FLAGS_CLAMP_RANGE: If set this indicates that the entry should be
>  *	flexibly clamp to min/max range in case the user provided an incorrect
>  *	value.
>  */
> enum ctl_table_flags {
> 	CTL_FLAGS_CLAMP_RANGE		= BIT(0),
> }
>
> This lets us document this nicely.

Thanks for the suggestion. Will update the code accordingly.

>> +
>>  struct ctl_node {
>>  	struct rb_node node;
>>  	struct ctl_table_header *header;
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 52b647a..2b2b30c 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2505,15 +2505,21 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>>   *
>>   * The do_proc_dointvec_minmax_conv_param structure provides the
>>   * minimum and maximum values for doing range checking for those sysctl
>> - * parameters that use the proc_dointvec_minmax() handler. The error
>> - * code -EINVAL will be returned if the range check fails.
>> + * parameters that use the proc_dointvec_minmax() handler.
>> + *
>> + * The error code -EINVAL will be returned if the range check fails
>> + * and the CTL_FLAGS_CLAMP_RANGE bit is not set in the given flags.
>> + * If that flag is set, the new sysctl value will be clamped to the
>> + * given range without returning any error.
> This last part seems odd, we silently set the value to a limit if the
> user set an invalid value?
>
> Since this is actually not really undefined documenting that we set it
> to the max value if the input value is greater than the max allowed would
> be good. Likewise for the minimum.
>
>   Luis

Will clarify the comment on that.

Cheers,
Longman

Re: [PATCH v2 3/5] sysctl: Warn when a clamped sysctl parameter is set out of range

[Waiman Long]

On 02/27/2018 07:57 PM, Luis R. Rodriguez wrote:
> On Tue, Feb 27, 2018 at 03:49:49PM -0500, Waiman Long wrote:
>> Even with clamped sysctl parameters, it is still not that straight
>> forward to figure out the exact range of those parameters. One may
>> try to write extreme parameter values to see if they get clamped.
>> To make it easier, a warning with the expected range will now be
>> printed in the kernel ring buffer when a clamped sysctl parameter
>> receives an out of range value.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>  include/linux/sysctl.h |  1 +
>>  kernel/sysctl.c        | 55 ++++++++++++++++++++++++++++++++++++++++++--------
>>  2 files changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index eceeaee..4e4f74a2 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -128,6 +128,7 @@ struct ctl_table
>>   * ctl_table flags (16 different flags, at most)
>>   */
>>  #define CTL_FLAGS_CLAMP_RANGE	(1 << 0) /* Clamp to min/max range */
>> +#define CTL_FLAGS_OOR_WARNED	(1 << 1) /* Out-of-range warning issued */
> With the enum set you can then kdocify this too.

Yes, will do.

>>  struct ctl_node {
>>  	struct rb_node node;
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 2b2b30c..f9f3373 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -2515,36 +2515,54 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
>>   *  min: ptr to minimum allowable value
>>   *  max: ptr to maximum allowable value
>>   *  flags: ptr to flags
>> + *  name: sysctl parameter name
>>   */
>>  struct do_proc_dointvec_minmax_conv_param {
>>  	int *min;
>>  	int *max;
>>  	uint16_t *flags;
>> +	const char *name;
>>  };
>>  
>>  static int do_proc_dointvec_minmax_conv(bool *negp, unsigned long *lvalp,
>>  					int *valp,
>>  					int write, void *data)
>>  {
>> +#define SYSCTL_WARN_MSG	\
>> +"Kernel parameter \"%s\" was set out of range [%d, %d], clamped to %d.\n"
> Please loose this define. What about a proc_ctl_warn() which takes the
> parameters and then does the checks?

Yes, I think that is better.

>> +
>>  	struct do_proc_douintvec_minmax_conv_param *param = data;
>>  
>>  	if (write) {
>>  		unsigned int val = *lvalp;
>> +		bool clamped = false;
>>  		bool clamp = param->flags &&
>>  			   (*param->flags & CTL_FLAGS_CLAMP_RANGE);
>>  
>> @@ -2623,24 +2649,36 @@ static int do_proc_douintvec_minmax_conv(unsigned long *lvalp,
>>  			return -EINVAL;
>>  
>>  		if (param->min && *param->min > val) {
>> -			if (clamp)
>> +			if (clamp) {
>>  				val = *param->min;
>> -			else
>> +				clamped = true;
>> +			} else {
>>  				return -ERANGE;
>> +			}
>>  		}
>>  		if (param->max && *param->max < val) {
>> -			if (clamp)
>> +			if (clamp) {
>>  				val = *param->max;
>> -			else
>> +				clamped = true;
>> +			} else {
>>  				return -ERANGE;
>> +			}
>>  		}
>>  		*valp = val;
>> +		if (clamped && param->name &&
>> +		   !(*param->flags & CTL_FLAGS_OOR_WARNED)) {
>> +			pr_warn(SYSCTL_WARN_MSG, param->name,
>> +				param->min ? *param->min : 0,
>> +				param->max ? *param->max : UINT_MAX, val);
>> +			*param->flags |= CTL_FLAGS_OOR_WARNED;
> Why not just use pr_warn_once()?
>
>   Luis

Right, pr_warn_once can be used in this case.

Cheers,
Longman

Re: [PATCH v2 4/5] ipc: Clamp msgmni and shmmni to the real IPCMNI limit

[Waiman Long]

On 02/27/2018 08:01 PM, Luis R. Rodriguez wrote:
> On Tue, Feb 27, 2018 at 03:49:50PM -0500, Waiman Long wrote:
>> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
>> index 8ad93c2..e4ab272 100644
>> --- a/ipc/ipc_sysctl.c
>> +++ b/ipc/ipc_sysctl.c
>> @@ -41,12 +41,17 @@ static int proc_ipc_dointvec(struct ctl_table *table, int write,
>>  static int proc_ipc_dointvec_minmax(struct ctl_table *table, int write,
>>  	void __user *buffer, size_t *lenp, loff_t *ppos)
>>  {
>> +	int ret;
>>  	struct ctl_table ipc_table;
>>  
>>  	memcpy(&ipc_table, table, sizeof(ipc_table));
>>  	ipc_table.data = get_ipc(table);
>>  
>> -	return proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
>> +	ret = proc_dointvec_minmax(&ipc_table, write, buffer, lenp, ppos);
>> +
>> +	table->flags |= ipc_table.flags; /* Copy back any change in flags */
> This seems fragile, why are we requiring this to be done by the users of
> CTL_FLAGS_CLAMP_RANGE ?
>
>   Luis

I should have just copied back the CTL_FLAGS_OOR_WARNED flag. That will
clarify what it is for.

Cheers,
Longman

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Luis R. Rodriguez]

On Wed, Feb 28, 2018 at 12:53:40PM -0500, Waiman Long wrote:
> On 02/27/2018 07:47 PM, Luis R. Rodriguez wrote:
> > On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
> >> When minimum/maximum values are specified for a sysctl parameter in
> >> the ctl_table structure with proc_dointvec_minmax() handler,
> > an
> >
> >> update
> >> to that parameter will fail with error if the given value is outside
> >> of the required range.
> >>
> >> There are use cases where it may be better to clamp the value of
> >> the sysctl parameter to the given range without failing the update,
> >> especially if the users are not aware of the actual range limits.
> > Makes me wonder if we should add something which does let one query
> > for the ranges. Then scripts can fetch that as well.
> 
> That will actually be better than printing out the range in the dmesg
> log. However, I haven't figured out an easy way of doing that. If you
> have any suggestion, please let me know about it.

I think a macro that also adds yet another proc read-only entry with a postfix
"_range" with an internal handler which prints the range may suffice.

  Luis

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Waiman Long]

On 02/28/2018 01:43 PM, Luis R. Rodriguez wrote:
> On Wed, Feb 28, 2018 at 12:53:40PM -0500, Waiman Long wrote:
>> On 02/27/2018 07:47 PM, Luis R. Rodriguez wrote:
>>> On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
>>>> When minimum/maximum values are specified for a sysctl parameter in
>>>> the ctl_table structure with proc_dointvec_minmax() handler,
>>> an
>>>
>>>> update
>>>> to that parameter will fail with error if the given value is outside
>>>> of the required range.
>>>>
>>>> There are use cases where it may be better to clamp the value of
>>>> the sysctl parameter to the given range without failing the update,
>>>> especially if the users are not aware of the actual range limits.
>>> Makes me wonder if we should add something which does let one query
>>> for the ranges. Then scripts can fetch that as well.
>> That will actually be better than printing out the range in the dmesg
>> log. However, I haven't figured out an easy way of doing that. If you
>> have any suggestion, please let me know about it.
> I think a macro that also adds yet another proc read-only entry with a postfix
> "_range" with an internal handler which prints the range may suffice.
>
>   Luis

I think that is a possible solution. Instead of adding a macro, I will
add one more flag which does the magic when the ctl_table entry is being
processed. I think that will be simpler from the user point of view.

Cheers,
Longman

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Luis R. Rodriguez]

On Wed, Feb 28, 2018 at 01:58:53PM -0500, Waiman Long wrote:
> On 02/28/2018 01:43 PM, Luis R. Rodriguez wrote:
> > On Wed, Feb 28, 2018 at 12:53:40PM -0500, Waiman Long wrote:
> >> On 02/27/2018 07:47 PM, Luis R. Rodriguez wrote:
> >>> On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
> >>>> When minimum/maximum values are specified for a sysctl parameter in
> >>>> the ctl_table structure with proc_dointvec_minmax() handler,
> >>> an
> >>>
> >>>> update
> >>>> to that parameter will fail with error if the given value is outside
> >>>> of the required range.
> >>>>
> >>>> There are use cases where it may be better to clamp the value of
> >>>> the sysctl parameter to the given range without failing the update,
> >>>> especially if the users are not aware of the actual range limits.
> >>> Makes me wonder if we should add something which does let one query
> >>> for the ranges. Then scripts can fetch that as well.
> >> That will actually be better than printing out the range in the dmesg
> >> log. However, I haven't figured out an easy way of doing that. If you
> >> have any suggestion, please let me know about it.
> > I think a macro that also adds yet another proc read-only entry with a postfix
> > "_range" with an internal handler which prints the range may suffice.
> >
> >   Luis
> 
> I think that is a possible solution. Instead of adding a macro, I will
> add one more flag which does the magic when the ctl_table entry is being
> processed. I think that will be simpler from the user point of view.

Agreed, flag should work nicely too. And likely easier to read / review
and maintain.

  Luis

Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping

[Waiman Long]

On 02/28/2018 01:58 PM, Waiman Long wrote:
> On 02/28/2018 01:43 PM, Luis R. Rodriguez wrote:
>> On Wed, Feb 28, 2018 at 12:53:40PM -0500, Waiman Long wrote:
>>> On 02/27/2018 07:47 PM, Luis R. Rodriguez wrote:
>>>> On Tue, Feb 27, 2018 at 03:49:48PM -0500, Waiman Long wrote:
>>>>> When minimum/maximum values are specified for a sysctl parameter in
>>>>> the ctl_table structure with proc_dointvec_minmax() handler,
>>>> an
>>>>
>>>>> update
>>>>> to that parameter will fail with error if the given value is outside
>>>>> of the required range.
>>>>>
>>>>> There are use cases where it may be better to clamp the value of
>>>>> the sysctl parameter to the given range without failing the update,
>>>>> especially if the users are not aware of the actual range limits.
>>>> Makes me wonder if we should add something which does let one query
>>>> for the ranges. Then scripts can fetch that as well.
>>> That will actually be better than printing out the range in the dmesg
>>> log. However, I haven't figured out an easy way of doing that. If you
>>> have any suggestion, please let me know about it.
>> I think a macro that also adds yet another proc read-only entry with a postfix
>> "_range" with an internal handler which prints the range may suffice.
>>
>>   Luis
> I think that is a possible solution. Instead of adding a macro, I will
> add one more flag which does the magic when the ctl_table entry is being
> processed. I think that will be simpler from the user point of view.
>
> Cheers,
> Longman
>
This patch will take a bit more time to work on. So I am going to do it
as a separate patch on top of the current one later. I don't want to
delay the review of the current patch set.

Cheers,
Longman