ipc: Clamp *mni to the real IPCMNI limit
Subsystems (2)
v2 cover ipc: Clamp *mni to the real IPCMNI limit
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(-)
Signature · 1 line
1.8.3.1
PATCH v2 1/5 sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param
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.
--- kernel/sysctl.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
kernel/sysctl.c +22 -0
@@ -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, ¶m); } +/** +* 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;
Signature · 1 line
1.8.3.1
Re: [PATCH v2 1/5] sysctl: Add kdoc comments to do_proc_do{u}intvec_minmax_conv_param
Quoted 11 lines · Waiman Long
+/**
+ * DOC:do_proc_dointvec_minmax_conv_param
+ *
+ * Thedo_proc_dointvec_minmax_conv_paramstructure provides the
+ * minimum and maximum values for doing range checking for those sysctl
+ * parameters that use theproc_dointvec_minmax()handler. The error
+ * code-EINVALwill 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
Quoted 23 lines · Matthew Wilcox
Quoted 11 lines · Waiman Long
+/**
+ * DOC:do_proc_dointvec_minmax_conv_param
+ *
+ * Thedo_proc_dointvec_minmax_conv_paramstructure provides the
+ * minimum and maximum values for doing range checking for those sysctl
+ * parameters that use theproc_dointvec_minmax()handler. The error
+ * code-EINVALwill 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/**
* structdo_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 toproc_dointvec_minmax(). */
Quoted 4 lines
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
PATCH v2 2/5 sysctl: Add flags to support min/max range clamping
In reply to Waiman LongWhen 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.
--- include/linux/sysctl.h | 6 ++++++ kernel/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 53 insertions(+), 11 deletions(-)
include/linux/sysctl.h +6 -0
@@ -116,6 +116,7 @@ struct ctl_tablevoid *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_tablevoid *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;
kernel/sysctl.c +47 -11
112 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Re: [PATCH v2 2/5] sysctl: Add flags to support min/max range clamping
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,
an
Quoted 7 lines
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.
Quoted 31 lines
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_tablestructure. The new field is a 16-bit value that just fits into the hole left by the 16-bitumode_tfield without increasing the size of the structure.When the
CTL_FLAGS_CLAMP_RANGEflag is set in thectl_tableentry, any update from the userspace will be clamped to the given range without error.Signed-off-by Waiman Long <[email protected]>--- include/linux/sysctl.h | 6 ++++++ kernel/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 53 insertions(+), 11 deletions(-)include/linux/sysctl.h +1
-0@@ -116,6 +116,7 @@ struct ctl_tablevoid *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;
include/linux/sysctl.h struct ctl_table · line 130
Quoted 7 lines
include/linux/sysctl.h +4
-0@@ -123,6 +124,11 @@ struct ctl_tablevoid *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.
Quoted 10 lines
include/linux/sysctl.h +1
-0+struct ctl_node {struct rb_node node;struct ctl_table_header *header;kernel/sysctl.c +0
-0@@ -2505,15 +2505,21 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,*
kernel/sysctl.c static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, · line 2513
Quoted 10 lines
kernel/sysctl.c +6
-2* 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 2/5] sysctl: Add flags to support min/max range clamping
Quoted 9 lines · Luis R. Rodriguez
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,an
Quoted 4 lines
update
to that parameter will fail with error if the given value is outside of the required range.
Quoted 5 lines
Quoted 3 lines
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.
Quoted 48 lines
Quoted 32 lines
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_tablestructure. The new field is a 16-bit value that just fits into the hole left by the 16-bitumode_tfield without increasing the size of the structure.When the
CTL_FLAGS_CLAMP_RANGEflag is set in thectl_tableentry, any update from the userspace will be clamped to the given range without error.Signed-off-by Waiman Long <[email protected]>--- include/linux/sysctl.h | 6 ++++++ kernel/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 53 insertions(+), 11 deletions(-)include/linux/sysctl.h +1
-0@@ -116,6 +116,7 @@ struct ctl_tablevoid *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;include/linux/sysctl.h struct ctl_table · line 130
Quoted 7 lines
include/linux/sysctl.h +4
-0@@ -123,6 +124,11 @@ struct ctl_tablevoid *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
/**
* enumctl_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.
*/
Quoted 5 lines
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.
Quoted 23 lines
Quoted 10 lines
include/linux/sysctl.h +1
-0+struct ctl_node {struct rb_node node;struct ctl_table_header *header;kernel/sysctl.c +0
-0@@ -2505,15 +2505,21 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,* kernel/sysctl.c static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, · line 2513
Quoted 10 lines
kernel/sysctl.c +6
-2* 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?
Quoted 5 lines
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 2/5] sysctl: Add flags to support min/max range clamping
Quoted 13 lines · Waiman Long
Quoted 12 lines · Luis R. Rodriguez
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,an
Quoted 7 lines
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.
Quoted 6 lines
Quoted 2 lines
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
Quoted 16 lines · Luis R. Rodriguez
Quoted 15 lines · Waiman Long
Quoted 14 lines · Luis R. Rodriguez
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,an
Quoted 7 lines
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.
Quoted 7 lines
Quoted 3 lines
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
Quoted 20 lines · Waiman Long
Quoted 19 lines · Luis R. Rodriguez
Quoted 18 lines · Waiman Long
Quoted 9 lines · Luis R. Rodriguez
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,an
Quoted 4 lines
update
to that parameter will fail with error if the given value is outside of the required range.Quoted 5 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 4 lines
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_tableentry 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
In reply to Waiman LongQuoted 23 lines · Waiman Long
Quoted 22 lines · Luis R. Rodriguez
Quoted 10 lines · Waiman Long
Quoted 9 lines · Luis R. Rodriguez
Quoted 2 lines · Waiman Long
When minimum/maximum values are specified for a sysctl parameter in the
ctl_tablestructure withproc_dointvec_minmax()handler,an
Quoted 4 lines
update
to that parameter will fail with error if the given value is outside of the required range.Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
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.
Quoted 8 lines
Quoted 1 line
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_tableentry 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
PATCH v2 3/5 sysctl: Warn when a clamped sysctl parameter is set out of range
In reply to Waiman LongEven 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.
--- include/linux/sysctl.h | 1 + kernel/sysctl.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 8 deletions(-)
include/linux/sysctl.h +1 -0
@@ -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;
kernel/sysctl.c +47 -8
146 lines not on this page · raw message
Signature · 1 line
1.8.3.1
Re: [PATCH v2 3/5] sysctl: Warn when a clamped sysctl parameter is set out of range
Quoted 18 lines · 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 <[email protected]>--- include/linux/sysctl.h | 1 + kernel/sysctl.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 8 deletions(-)include/linux/sysctl.h +0
-0
include/linux/sysctl.h struct ctl_table · line 131
Quoted 4 lines
include/linux/sysctl.h +1
-0@@ -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.
Quoted 15 lines
include/linux/sysctl.h +0
-0struct ctl_node {struct rb_node node;kernel/sysctl.c +1
-0@@ -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;
kernel/sysctl.c static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, · line 2532
Quoted 10 lines
kernel/sysctl.c +3
-0uint16_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?
Quoted 29 lines
kernel/sysctl.c +9
-4+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 {
kernel/sysctl.c static int do_proc_douintvec_minmax_conv(unsigned long *lvalp, · line 2673
Quoted 10 lines
kernel/sysctl.c +7
-0return -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 3/5] sysctl: Warn when a clamped sysctl parameter is set out of range
Quoted 14 lines · Luis R. Rodriguez
Quoted 13 lines · 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 <[email protected]>--- include/linux/sysctl.h | 1 + kernel/sysctl.c | 55 ++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 8 deletions(-)
Quoted 10 lines
include/linux/sysctl.h struct ctl_table · line 131
Quoted 9 lines
include/linux/sysctl.h +1
-0@@ -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.
Quoted 19 lines
Quoted 19 lines
include/linux/sysctl.h +0
-0struct ctl_node {struct rb_node node;kernel/sysctl.c +2
-0@@ -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; };
Quoted 8 lines
kernel/sysctl.c static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write, · line 2532
Quoted 6 lines
kernel/sysctl.c +2
-0static 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.
Quoted 41 lines
Quoted 29 lines
kernel/sysctl.c +9
-4+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 {kernel/sysctl.c static int do_proc_douintvec_minmax_conv(unsigned long *lvalp, · line 2673
Quoted 10 lines
kernel/sysctl.c +7
-0return -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()?
Quoted 1 line
Luis
Right, pr_warn_once can be used in this case.
Cheers,
Longman
PATCH v2 4/5 ipc: Clamp msgmni and shmmni to the real IPCMNI limit
In reply to Waiman LongA 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.
--- ipc/ipc_sysctl.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
ipc/ipc_sysctl.c +13 -3
@@ -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",
Signature · 1 line
1.8.3.1
Re: [PATCH v2 4/5] ipc: Clamp msgmni and shmmni to the real IPCMNI limit
Quoted 8 lines · Waiman Long
ipc/ipc_sysctl.c +0
-0@@ -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) {
ipc/ipc_sysctl.c static int proc_ipc_dointvec(struct ctl_table *table, int write, · line 52
Quoted 10 lines
ipc/ipc_sysctl.c +4
-1+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 4/5] ipc: Clamp msgmni and shmmni to the real IPCMNI limit
Quoted 15 lines · Luis R. Rodriguez
Quoted 14 lines · Waiman Long
ipc/ipc_sysctl.c +1
-0@@ -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);
Quoted 8 lines
ipc/ipc_sysctl.c static int proc_ipc_dointvec(struct ctl_table *table, int write, · line 52
Quoted 4 lines
ipc/ipc_sysctl.c +3
-1-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
PATCH v2 5/5 ipc: Clamp semmni to the real IPCMNI limit
In reply to Waiman LongThis 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.
--- ipc/ipc_sysctl.c | 13 ++++++++++++- ipc/sem.c | 33 +++++++++++++++++++++++++++++++++ ipc/util.h | 4 ++++ 3 files changed, 49 insertions(+), 1 deletion(-)
ipc/ipc_sysctl.c +12 -1
@@ -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 {
ipc/sem.c +33 -0
@@ -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
ipc/util.h +4 -0
@@ -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 {
Signature · 1 line
1.8.3.1