| Message ID | 1519764591-27456-5-git-send-email-longman@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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",
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(-)