netdev - Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Sep 2016 19:08:44 +0900
From:   Maciej Żenczykowski <maze@...gle.com>
To:     Hannes Frederic Sowa <hannes@...essinduktion.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Linux NetDev <netdev@...r.kernel.org>,
        Erik Kline <ek@...gle.com>,
        Lorenzo Colitti <lorenzo@...gle.com>
Subject: Re: [PATCH v4 4/7] ipv6 addrconf: add new sysctl 'router_solicitation_max_interval'

> Good point. Using ms should actually be easy, instead of
> proc_dointvec_jiffies you can use proc_dointvec_ms_jiffies.

Yes, I'm aware of this, but 'proc_dointvec_ms_jiffies' seems to be a
bit of a hack, and especially for large settings like this exporting
them to userspace in units of ms instead of seconds seems to impose
unnecessary cognitive burden on the user.

(furthermore the other two related settings are exported as seconds)

> It seems good practice to add a _ms then to the sysctl, too.

If we wanted to do that, we'd have to add _ms versions of
router_solicitation_{delay,interval}.
This seems like pointless duplication - since we can't actually delete
the older seconds one.

There isn't really any real world scenario I can think of (besides
unit-testing) where ms granularity would be useful.

If we were to actually fix proc_dointvec_jiffies to accept fractional
seconds, we'd fix all of these interfaces in one fell swoop...

I took a look at what this would take, and there doesn't really appear
to be a nice way to do it :-(
The lower level conversion functions get a character buffer and
convert it to/from an int (or maybe long).

We'd either need to switch these over to take a void* or add an
entirely separate implementation of proc_dointvec_jiffies...

(at which point it could probably also parse units (ns, us, ms, s, m,
h, d) on input if we wanted to...)

> I am fine with both ways.

I'm going to leave this as is (especially since I send out v5 before I
saw your comment)