[PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

[PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

We perform the conversion between kernel jiffies and ms only
when exporting kernel value to user space.

We need to do the opposite operation when value is written
by user.

Only matters when HZ != 1000

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: stable@vger.kernel.org
---
 kernel/sysctl.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8dbaec0e4f7f079b87f50ea67c82341304387783..1aea594a54dbdac604ca950fdaf93508e5b6e6a7 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -2475,6 +2475,7 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int
 				break;
 			if (neg)
 				continue;
+			val = convmul * val / convdiv;
 			if ((min && val < *min) || (max && val > *max))
 				continue;
 			*i = val;

Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

[Linus Torvalds]

On Wed, Jan 25, 2017 at 6:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> We perform the conversion between kernel jiffies and ms only
> when exporting kernel value to user space.
>
> We need to do the opposite operation when value is written
> by user.

Applied. The interface is really confusing - I had to check whether
the min/max values were in jiffies or in ms, but it looks right. It
would probably be good to verify any overflow conditions, but we do
check the min/max after the conversion, so overflows are at least only
an inconvenience, not a "you can set invalid values".

The one overflow case I can imagine is trying to set some unlimited
value, and now due to the conversion math it's not unlimited any more,
and it just happened to work.

But the current code is clearly wrong. Of course, something might end
up depending on it being wrong, so..

           Linus

Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

[Eric Dumazet]

On Thu, 2017-01-26 at 09:25 -0800, Linus Torvalds wrote:
> On Wed, Jan 25, 2017 at 6:20 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > We perform the conversion between kernel jiffies and ms only
> > when exporting kernel value to user space.
> >
> > We need to do the opposite operation when value is written
> > by user.
> 
> Applied. The interface is really confusing - I had to check whether
> the min/max values were in jiffies or in ms, but it looks right. It
> would probably be good to verify any overflow conditions, but we do
> check the min/max after the conversion, so overflows are at least only
> an inconvenience, not a "you can set invalid values".
> 
> The one overflow case I can imagine is trying to set some unlimited
> value, and now due to the conversion math it's not unlimited any more,
> and it just happened to work.
> 
> But the current code is clearly wrong. Of course, something might end
> up depending on it being wrong, so..

Yes, these 'convdiv' and 'convmul' names are really confusing,
especially considering we either do :

val = convmul * val / convdiv;

or

val = convdiv * (*i) / convmul;

Probably should be renamed to something like :

user_units  kernel_units

Maybe.

Thanks.