Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1567086 > unrolled thread

[PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

Back to article view | Back to linux.kernel

Contents

  [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax() Eric Dumazet <eric.dumazet@gmail.com> - 2017-01-26 03:30 +0100
    Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax() Linus Torvalds <torvalds@linux-foundation.org> - 2017-01-26 18:40 +0100
      Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax() Eric Dumazet <eric.dumazet@gmail.com> - 2017-01-26 19:10 +0100

#1567086 — [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

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;

[toc] | [next] | [standalone]

#1567623

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

[toc] | [prev] | [next] | [standalone]

#1567638

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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web