linux-kernel - Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1485453979.5145.147.camel@edumazet-glaptop3.roam.corp.google.com>
Date:   Thu, 26 Jan 2017 10:06:19 -0800
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sysctl: fix proc_doulongvec_ms_jiffies_minmax()

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@...il.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.