| Message ID | 20190628074230.838-1-po-hsu.lin@canonical.com |
|---|---|
| Headers | show |
| Series | sysctl: handle overflow in proc_get_long | expand |
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
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... Acked-by: Stefan Bader <stefan.bader@canonical.com>
On 6/28/19 9:42 AM, 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(-) > Applied to {xenial,bionic,cosmic,disco}/master-next branches. Thanks, Kleber
On Fri, Jun 28, 2019 at 03:42:27PM +0800, 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. Patch is from 5.2-rc1, so unstable already has it.
On Mon, Jul 1, 2019 at 5:31 PM Stefan Bader <stefan.bader@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... > Hi Stefan, 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[1]: .... if the same patch can't be applied to the different trees it needs to be split up. Thanks Sam [1] https://lists.ubuntu.com/archives/kernel-team/2019-March/099068.html > Acked-by: Stefan Bader <stefan.bader@canonical.com> >
== 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(-)