ACK/Cmnt: [X][B][C][D][Unstable][SRU][PATCH 0/1] sysctl: handle overflow in proc_get_long
po-hsu.lin at canonical.com
Wed Jul 17 02:59:38 UTC 2019
On Mon, Jul 1, 2019 at 5:31 PM Stefan Bader <stefan.bader at canonical.com> wrote:
> On 28.06.19 09:42, Po-Hsu Lin wrote:
> > == SRU Justification ==
> > With the upper / lower boundary confined in bug 1834310, the file-max
> > is still suffering with overflow issue.
> > This is because the simple_strtoul() used in proc_get_long() to parse
> > user input explicitly ignores overflows. So when you tried to put 2^64
> > into file-max, it will:
> > # echo 18446744073709551616 > /proc/sys/fs/file-max
> > # cat /proc/sys/fs/file-max
> > 0
> > Which will cause your system to silently die behind your back.
> > This issue was reported by the case 1 of the sysctl02 test in LTP:
> > sysctl02 1 TFAIL: /proc/sys/fs/file-max overflows and set to 0
> > == Fix ==
> > * 7f2923c4 (sysctl: handle overflow in proc_get_long)
> > A new strtoul_lenient() was introduced here to solve this issue, with
> > extra check to notify userspace with -EINVAL.
> > This patch can be cherry-picked into B/C/D/E, it needs some content
> > adjustment for X.
> > == Test ==
> > Test kernels could be found here:
> > https://people.canonical.com/~phlin/kernel/lp-1833935-proc_get_long/
> > The attempt to set file-max to 2^64 will be rejected:
> > $ sudo sysctl -w -q fs.file-max=18446744073709551616
> > sysctl: setting key "fs.file-max": Invalid argument
> > Tested and passed with these kernels on AMD64 KVM nodes.
> > == Regression Potential ==
> > Low, the newly introduced function strtoul_lenient() is just for
> > proc_get_long here.
> > Christian Brauner (1):
> > sysctl: handle overflow in proc_get_long
> > kernel/sysctl.c | 40 +++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 39 insertions(+), 1 deletion(-)
> If this can be cherry picked into B/C/D and unstable, why did you sent that
> patch twice? Anyhow...
sorry for the late reply,
it's because the patch generated for B/C cannot be applied to
D/Unstable with "git am"
This was based on Kleber's comment:
.... if the same patch can't be applied to the different trees it
needs to be split up.
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
More information about the kernel-team