| Message ID | 20250224095826.16458-1-nicolas.bouchinet@clip-os.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixes multiple sysctl bound checks | expand |
On Mon, Feb 24, 2025 at 10:58:15AM +0100, nicolas.bouchinet@clip-os.org wrote: > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> > > Hi, > > This patchset adds some bound checks to sysctls to avoid negative > value writes. > > The patched sysctls were storing the result of the proc_dointvec > proc_handler into an unsigned int data. proc_dointvec being able to > parse negative value, and it return value being a signed int, this could > lead to undefined behaviors. > This has led to kernel crash in the past as described in commit > 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in vm_table") This patch is slightly different then what you are trying to do here. Notice that the issue in 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in vm_table") was that the extra1 value was getting ignored because it was calling proc_dointvec; replacing it with proc_dointvec_minmax properly forwards the extra1 value. What your series is trying to do is to avoid a assignment of a negative value to a unsigned int. > > They are now bounded between SYSCTL_ZERO and SYSCTL_INT_MAX. > The proc_handlers have thus been updated to proc_dointvec_minmax. > > This patchset has been written over sysctl-testing branch [1]. > See [2] for similar sysctl fixes currently in review. > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/log/?h=sysctl-testing > [2]: https://lore.kernel.org/all/20250115132211.25400-1-nicolas.bouchinet@clip-os.org/ > > Best regards, > > Nicolas In general, I would like for these patches to go into mainline through their individual subsystem as they would know more about what type fits best for the ctl_table->data variable. With that said, I'll push the leftovers through sysctl if there are no takers. Thx for the series > > --- > > Changes since v1: > https://lore.kernel.org/all/20250127142014.37834-1-nicolas.bouchinet@clip-os.org/ > > * Detached patches 1/9, 2/9 [3] and 3/9 [4] > * Adapted the cover-letter message to match the reduced patchset > > [3]: https://lore.kernel.org/all/20250129170633.88574-1-nicolas.bouchinet@clip-os.org/ > [4]: https://lore.kernel.org/all/20250128103821.29745-1-nicolas.bouchinet@clip-os.org/ > > --- > > Nicolas Bouchinet (6): > sysctl: Fixes idmap_cache_timeout bounds > sysctl: Fixes nsm_local_state bounds > sysctl/coda: Fixes timeout bounds > sysctl: Fixes scsi_logging_level bounds > sysctl/infiniband: Fixes infiniband sysctl bounds > sysctl: Fixes max-user-freq bounds > > drivers/char/hpet.c | 4 +++- > drivers/infiniband/core/iwcm.c | 4 +++- > drivers/infiniband/core/ucma.c | 4 +++- > drivers/scsi/scsi_sysctl.c | 4 +++- > fs/coda/sysctl.c | 4 +++- > fs/lockd/svc.c | 4 +++- > fs/nfs/nfs4sysctl.c | 4 +++- > 7 files changed, 21 insertions(+), 7 deletions(-) > > -- > 2.48.1 >
On Mon, 24 Feb 2025 10:58:15 +0100, nicolas.bouchinet@clip-os.org wrote: > This patchset adds some bound checks to sysctls to avoid negative > value writes. > > The patched sysctls were storing the result of the proc_dointvec > proc_handler into an unsigned int data. proc_dointvec being able to > parse negative value, and it return value being a signed int, this could > lead to undefined behaviors. > This has led to kernel crash in the past as described in commit > 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in vm_table") > > [...] Applied to 6.15/scsi-queue, thanks! [4/6] sysctl: Fixes scsi_logging_level bounds https://git.kernel.org/mkp/scsi/c/2cef5b4472c6
From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> Hi, This patchset adds some bound checks to sysctls to avoid negative value writes. The patched sysctls were storing the result of the proc_dointvec proc_handler into an unsigned int data. proc_dointvec being able to parse negative value, and it return value being a signed int, this could lead to undefined behaviors. This has led to kernel crash in the past as described in commit 3b3376f222e3 ("sysctl.c: fix underflow value setting risk in vm_table") They are now bounded between SYSCTL_ZERO and SYSCTL_INT_MAX. The proc_handlers have thus been updated to proc_dointvec_minmax. This patchset has been written over sysctl-testing branch [1]. See [2] for similar sysctl fixes currently in review. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/sysctl/sysctl.git/log/?h=sysctl-testing [2]: https://lore.kernel.org/all/20250115132211.25400-1-nicolas.bouchinet@clip-os.org/ Best regards, Nicolas --- Changes since v1: https://lore.kernel.org/all/20250127142014.37834-1-nicolas.bouchinet@clip-os.org/ * Detached patches 1/9, 2/9 [3] and 3/9 [4] * Adapted the cover-letter message to match the reduced patchset [3]: https://lore.kernel.org/all/20250129170633.88574-1-nicolas.bouchinet@clip-os.org/ [4]: https://lore.kernel.org/all/20250128103821.29745-1-nicolas.bouchinet@clip-os.org/ --- Nicolas Bouchinet (6): sysctl: Fixes idmap_cache_timeout bounds sysctl: Fixes nsm_local_state bounds sysctl/coda: Fixes timeout bounds sysctl: Fixes scsi_logging_level bounds sysctl/infiniband: Fixes infiniband sysctl bounds sysctl: Fixes max-user-freq bounds drivers/char/hpet.c | 4 +++- drivers/infiniband/core/iwcm.c | 4 +++- drivers/infiniband/core/ucma.c | 4 +++- drivers/scsi/scsi_sysctl.c | 4 +++- fs/coda/sysctl.c | 4 +++- fs/lockd/svc.c | 4 +++- fs/nfs/nfs4sysctl.c | 4 +++- 7 files changed, 21 insertions(+), 7 deletions(-)