[PATCH v2 0/6] Fixes multiple sysctl bound checks

[PATCH v2 0/6] Fixes multiple sysctl bound checks

[nicolas.bouchinet]

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(-)

-- 
2.48.1

[PATCH v2 1/6] sysctl: Fixes idmap_cache_timeout bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound idmap_cache_timeout sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 fs/nfs/nfs4sysctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/nfs4sysctl.c b/fs/nfs/nfs4sysctl.c
index d1a92d8f8ba4c..c36d89afb13af 100644
--- a/fs/nfs/nfs4sysctl.c
+++ b/fs/nfs/nfs4sysctl.c
@@ -32,7 +32,9 @@ static const struct ctl_table nfs4_cb_sysctls[] = {
 		.data = &nfs_idmap_cache_timeout,
 		.maxlen = sizeof(int),
 		.mode = 0644,
-		.proc_handler = proc_dointvec,
+		.proc_handler = proc_dointvec_minmax,
+		.extra1 = SYSCTL_ZERO,
+		.extra2 = SYSCTL_INT_MAX,
 	},
 };
 
-- 
2.48.1

[PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound nsm_local_state sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 fs/lockd/svc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
index 2c8eedc6c2cc9..984ab233af8b6 100644
--- a/fs/lockd/svc.c
+++ b/fs/lockd/svc.c
@@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = {
 		.data		= &nsm_local_state,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 };
 
-- 
2.48.1

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[Chuck Lever]

On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> Bound nsm_local_state sysctl writings between SYSCTL_ZERO
> and SYSCTL_INT_MAX.
> 
> The proc_handler has thus been updated to proc_dointvec_minmax.
> 
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>  fs/lockd/svc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> index 2c8eedc6c2cc9..984ab233af8b6 100644
> --- a/fs/lockd/svc.c
> +++ b/fs/lockd/svc.c
> @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = {
>  		.data		= &nsm_local_state,
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
> +		.extra2		= SYSCTL_INT_MAX,
>  	},
>  };
>  

Hi Nicolas -

nsm_local_state is an unsigned 32-bit integer. The type of that value is
defined by spec, because this value is exchanged between peers on the
network.

Perhaps this patch should replace proc_dointvec with proc_douintvec
instead.


-- 
Chuck Lever

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[Nicolas Bouchinet]

On 2/24/25 15:38, Chuck Lever wrote:
> On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote:
>> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>>
>> Bound nsm_local_state sysctl writings between SYSCTL_ZERO
>> and SYSCTL_INT_MAX.
>>
>> The proc_handler has thus been updated to proc_dointvec_minmax.
>>
>> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>> ---
>>   fs/lockd/svc.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>> index 2c8eedc6c2cc9..984ab233af8b6 100644
>> --- a/fs/lockd/svc.c
>> +++ b/fs/lockd/svc.c
>> @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = {
>>   		.data		= &nsm_local_state,
>>   		.maxlen		= sizeof(int),
>>   		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> +		.proc_handler	= proc_dointvec_minmax,
>> +		.extra1		= SYSCTL_ZERO,
>> +		.extra2		= SYSCTL_INT_MAX,
>>   	},
>>   };
>>   
> Hi Nicolas -
>
> nsm_local_state is an unsigned 32-bit integer. The type of that value is
> defined by spec, because this value is exchanged between peers on the
> network.
>
> Perhaps this patch should replace proc_dointvec with proc_douintvec
> instead.
>
>

Hi Chuck,

Thank's for your review.

If `nsm_local_state` should be set to the
full range of an uint32_t by a user writing in the sysctl, then yes it 
should
use `proc_douintvec` instead of limiting it to SYSCTL_INT_MAX value 
(INT_MAX).

I've used `proc_dointvec_minmax` since it already used `proc_dointvec` 
and thus
was already capped at INT_MAX.

Best regards,

Nicolas

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[Joel Granados]

On Mon, Feb 24, 2025 at 09:38:17AM -0500, Chuck Lever wrote:
> On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > Bound nsm_local_state sysctl writings between SYSCTL_ZERO
> > and SYSCTL_INT_MAX.
> > 
> > The proc_handler has thus been updated to proc_dointvec_minmax.
> > 
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  fs/lockd/svc.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
> > index 2c8eedc6c2cc9..984ab233af8b6 100644
> > --- a/fs/lockd/svc.c
> > +++ b/fs/lockd/svc.c
> > @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = {
> >  		.data		= &nsm_local_state,
> >  		.maxlen		= sizeof(int),
> >  		.mode		= 0644,
> > -		.proc_handler	= proc_dointvec,
> > +		.proc_handler	= proc_dointvec_minmax,
> > +		.extra1		= SYSCTL_ZERO,
> > +		.extra2		= SYSCTL_INT_MAX,
> >  	},
> >  };
> >  
> 
> Hi Nicolas -
> 
> nsm_local_state is an unsigned 32-bit integer. The type of that value is
> defined by spec, because this value is exchanged between peers on the
> network.
> 
> Perhaps this patch should replace proc_dointvec with proc_douintvec
> instead.
As Nicolas stated, that is completely up to how you used the variable.

Things to notice:
1. If you want the full range of a unsigned long, then you should stop
   using proc_dointvec as it will upper limit the value to INT_MAX.
2. If you want to keep using nsm_local_state as unsigned int, then
   please add SYSCTL_ZERO as a lower bound to avoid assigning negative
   values
3. Having SYSCTL_INT_MAX is not necessary as it is already capped by
   proc_dointvec{_minmax,}, but it is nice to have as it makes explicit
   what is happening.

Let me know if you take this through your trees so I can remove it from
sysctl.

Reviewed-by: Joel Granados <joel.granados@kernel.org>

Best

> 
> 
> -- 
> Chuck Lever

-- 

Joel Granados

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[Chuck Lever]

On 3/3/25 9:12 AM, Joel Granados wrote:
> On Mon, Feb 24, 2025 at 09:38:17AM -0500, Chuck Lever wrote:
>> On 2/24/25 4:58 AM, nicolas.bouchinet@clip-os.org wrote:
>>> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>>>
>>> Bound nsm_local_state sysctl writings between SYSCTL_ZERO
>>> and SYSCTL_INT_MAX.
>>>
>>> The proc_handler has thus been updated to proc_dointvec_minmax.
>>>
>>> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>>> ---
>>>  fs/lockd/svc.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/lockd/svc.c b/fs/lockd/svc.c
>>> index 2c8eedc6c2cc9..984ab233af8b6 100644
>>> --- a/fs/lockd/svc.c
>>> +++ b/fs/lockd/svc.c
>>> @@ -461,7 +461,9 @@ static const struct ctl_table nlm_sysctls[] = {
>>>  		.data		= &nsm_local_state,
>>>  		.maxlen		= sizeof(int),
>>>  		.mode		= 0644,
>>> -		.proc_handler	= proc_dointvec,
>>> +		.proc_handler	= proc_dointvec_minmax,
>>> +		.extra1		= SYSCTL_ZERO,
>>> +		.extra2		= SYSCTL_INT_MAX,
>>>  	},
>>>  };
>>>  
>>
>> Hi Nicolas -
>>
>> nsm_local_state is an unsigned 32-bit integer. The type of that value is
>> defined by spec, because this value is exchanged between peers on the
>> network.
>>
>> Perhaps this patch should replace proc_dointvec with proc_douintvec
>> instead.
> As Nicolas stated, that is completely up to how you used the variable.
> 
> Things to notice:
> 1. If you want the full range of a unsigned long, then you should stop
>    using proc_dointvec as it will upper limit the value to INT_MAX.
> 2. If you want to keep using nsm_local_state as unsigned int, then
>    please add SYSCTL_ZERO as a lower bound to avoid assigning negative
>    values
> 3. Having SYSCTL_INT_MAX is not necessary as it is already capped by
>    proc_dointvec{_minmax,}, but it is nice to have as it makes explicit
>    what is happening.
> 
> Let me know if you take this through your trees so I can remove it from
> sysctl.
> 
> Reviewed-by: Joel Granados <joel.granados@kernel.org>

The variable "nsm_local_state" is declared as "u32".

The range of the value of nsm_local_state is supposed to be 0 -
UINT_MAX (ie, an unsigned 32-bit integer), so I'm proposing:

 		.data		= &nsm_local_state,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_douintvec,
 	},
 };

But I suspect that the "maxlen" field should be changed to something
like "sizeof(nsm_local_state)".

Does that seem correct to you?

I can take this patch through the NFSD tree so it can get some NFS-
specific testing.

-- 
Chuck Lever

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

On Mon, 24 Feb 2025 10:58:17 +0100, nicolas.bouchinet@clip-os.org wrote:
> Bound nsm_local_state sysctl writings between SYSCTL_ZERO
> and SYSCTL_INT_MAX.
> 
> The proc_handler has thus been updated to proc_dointvec_minmax.
> 
> 

Applied to nfsd-testing with modifications, thanks!

[2/6] sysctl: Fixes nsm_local_state bounds
      commit: 8e6d33ea0159b39d670b7986324bd6135ee9d5f7

--
Chuck Lever

Re: [PATCH v2 2/6] sysctl: Fixes nsm_local_state bounds

[Joel Granados]

On Mon, Mar 03, 2025 at 04:42:17PM -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> On Mon, 24 Feb 2025 10:58:17 +0100, nicolas.bouchinet@clip-os.org wrote:
> > Bound nsm_local_state sysctl writings between SYSCTL_ZERO
> > and SYSCTL_INT_MAX.
> > 
> > The proc_handler has thus been updated to proc_dointvec_minmax.
> > 
> > 
> 
> Applied to nfsd-testing with modifications, thanks!
> 
> [2/6] sysctl: Fixes nsm_local_state bounds
>       commit: 8e6d33ea0159b39d670b7986324bd6135ee9d5f7
> 
> --
> Chuck Lever
Thx!

@nicolas: remove this one from your next Version

Best

-- 

Joel Granados

[PATCH v2 3/6] sysctl/coda: Fixes timeout bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound coda timeout sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 fs/coda/sysctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/coda/sysctl.c b/fs/coda/sysctl.c
index 0df46f09b6cc5..d6f8206c51575 100644
--- a/fs/coda/sysctl.c
+++ b/fs/coda/sysctl.c
@@ -20,7 +20,9 @@ static const struct ctl_table coda_table[] = {
 		.data		= &coda_timeout,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 	{
 		.procname	= "hard",
-- 
2.48.1

Re: [PATCH v2 3/6] sysctl/coda: Fixes timeout bounds

[Joel Granados]

On Mon, Feb 24, 2025 at 10:58:18AM +0100, nicolas.bouchinet@clip-os.org wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> Bound coda timeout sysctl writings between SYSCTL_ZERO
> and SYSCTL_INT_MAX.
> 
> The proc_handler has thus been updated to proc_dointvec_minmax.
> 
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>  fs/coda/sysctl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/coda/sysctl.c b/fs/coda/sysctl.c
> index 0df46f09b6cc5..d6f8206c51575 100644
> --- a/fs/coda/sysctl.c
> +++ b/fs/coda/sysctl.c
> @@ -20,7 +20,9 @@ static const struct ctl_table coda_table[] = {
>  		.data		= &coda_timeout,
I noticed that coda_timeout is an unsigned long. With that in mind I
would change it to unsigned int. It seems to be a value that can be
ranged within [0,INT_MAX]

Best
>  		.maxlen		= sizeof(int),
>  		.mode		= 0644,
> -		.proc_handler	= proc_dointvec
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
> +		.extra2		= SYSCTL_INT_MAX,
>  	},
>  	{
>  		.procname	= "hard",
> -- 
> 2.48.1
> 

-- 

Joel Granados

Re: [PATCH v2 3/6] sysctl/coda: Fixes timeout bounds

[Jan Harkes]

On Mon, Mar 03, 2025 at 09:16:10AM -0500, Joel Granados wrote:
> On Mon, Feb 24, 2025 at 10:58:18AM +0100, nicolas.bouchinet@clip-os.org wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > Bound coda timeout sysctl writings between SYSCTL_ZERO
> > and SYSCTL_INT_MAX.
> > 
> > The proc_handler has thus been updated to proc_dointvec_minmax.
> > 
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  fs/coda/sysctl.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/coda/sysctl.c b/fs/coda/sysctl.c
> > index 0df46f09b6cc5..d6f8206c51575 100644
> > --- a/fs/coda/sysctl.c
> > +++ b/fs/coda/sysctl.c
> > @@ -20,7 +20,9 @@ static const struct ctl_table coda_table[] = {
> >  		.data		= &coda_timeout,
> I noticed that coda_timeout is an unsigned long. With that in mind I
> would change it to unsigned int. It seems to be a value that can be
> ranged within [0,INT_MAX]

That seems fine by me.

It is a timeout in seconds and it is typically set to some value well
under a minute.

Jan

Re: [PATCH v2 3/6] sysctl/coda: Fixes timeout bounds

[Joel Granados]

On Mon, Mar 03, 2025 at 09:39:37AM -0500, Jan Harkes wrote:
> On Mon, Mar 03, 2025 at 09:16:10AM -0500, Joel Granados wrote:
> > On Mon, Feb 24, 2025 at 10:58:18AM +0100, nicolas.bouchinet@clip-os.org wrote:
> > > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > 
> > > Bound coda timeout sysctl writings between SYSCTL_ZERO
> > > and SYSCTL_INT_MAX.
> > > 
> > > The proc_handler has thus been updated to proc_dointvec_minmax.
> > > 
> > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > ---
> > >  fs/coda/sysctl.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/coda/sysctl.c b/fs/coda/sysctl.c
> > > index 0df46f09b6cc5..d6f8206c51575 100644
> > > --- a/fs/coda/sysctl.c
> > > +++ b/fs/coda/sysctl.c
> > > @@ -20,7 +20,9 @@ static const struct ctl_table coda_table[] = {
> > >  		.data		= &coda_timeout,
> > I noticed that coda_timeout is an unsigned long. With that in mind I
> > would change it to unsigned int. It seems to be a value that can be
> > ranged within [0,INT_MAX]
> 
> That seems fine by me.
> 
> It is a timeout in seconds and it is typically set to some value well
> under a minute.
> 

Thx for the confirmation. I'll let nicolas take care of the change
Best

-- 

Joel Granados

Re: [PATCH v2 3/6] sysctl/coda: Fixes timeout bounds

[Nicolas Bouchinet]

Hi Jan and Joel,

Thanks for your review,

I'll update coda_timeout type to an unsigned int and push back the patchset
without patches that has moved in their subsystems.

Best regards,

Nicolas

On 3/5/25 15:47, Joel Granados wrote:
> On Mon, Mar 03, 2025 at 09:39:37AM -0500, Jan Harkes wrote:
>> On Mon, Mar 03, 2025 at 09:16:10AM -0500, Joel Granados wrote:
>>> On Mon, Feb 24, 2025 at 10:58:18AM +0100, nicolas.bouchinet@clip-os.org wrote:
>>>> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>>>>
>>>> Bound coda timeout sysctl writings between SYSCTL_ZERO
>>>> and SYSCTL_INT_MAX.
>>>>
>>>> The proc_handler has thus been updated to proc_dointvec_minmax.
>>>>
>>>> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>>>> ---
>>>>   fs/coda/sysctl.c | 4 +++-
>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/coda/sysctl.c b/fs/coda/sysctl.c
>>>> index 0df46f09b6cc5..d6f8206c51575 100644
>>>> --- a/fs/coda/sysctl.c
>>>> +++ b/fs/coda/sysctl.c
>>>> @@ -20,7 +20,9 @@ static const struct ctl_table coda_table[] = {
>>>>   		.data		= &coda_timeout,
>>> I noticed that coda_timeout is an unsigned long. With that in mind I
>>> would change it to unsigned int. It seems to be a value that can be
>>> ranged within [0,INT_MAX]
>> That seems fine by me.
>>
>> It is a timeout in seconds and it is typically set to some value well
>> under a minute.
>>
> Thx for the confirmation. I'll let nicolas take care of the change
> Best
>

[PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound scsi_logging_level sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 drivers/scsi/scsi_sysctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_sysctl.c b/drivers/scsi/scsi_sysctl.c
index be4aef0f4f996..055a03a83ad68 100644
--- a/drivers/scsi/scsi_sysctl.c
+++ b/drivers/scsi/scsi_sysctl.c
@@ -17,7 +17,9 @@ static const struct ctl_table scsi_table[] = {
 	  .data		= &scsi_logging_level,
 	  .maxlen	= sizeof(scsi_logging_level),
 	  .mode		= 0644,
-	  .proc_handler	= proc_dointvec },
+	  .proc_handler	= proc_dointvec_minmax,
+	  .extra1	= SYSCTL_ZERO,
+	  .extra2	= SYSCTL_INT_MAX },
 };
 
 static struct ctl_table_header *scsi_table_header;
-- 
2.48.1

Re: [PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[Martin K. Petersen]

Hi Nicolas!

> --- a/drivers/scsi/scsi_sysctl.c
> +++ b/drivers/scsi/scsi_sysctl.c
> @@ -17,7 +17,9 @@ static const struct ctl_table scsi_table[] = {
>  	  .data		= &scsi_logging_level,
>  	  .maxlen	= sizeof(scsi_logging_level),
>  	  .mode		= 0644,
> -	  .proc_handler	= proc_dointvec },
> +	  .proc_handler	= proc_dointvec_minmax,
> +	  .extra1	= SYSCTL_ZERO,
> +	  .extra2	= SYSCTL_INT_MAX },

scsi_logging_level is a bitmask and should be unsigned.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[Nicolas Bouchinet]

On 2/25/25 02:20, Martin K. Petersen wrote:
> Hi Nicolas!
>
>> --- a/drivers/scsi/scsi_sysctl.c
>> +++ b/drivers/scsi/scsi_sysctl.c
>> @@ -17,7 +17,9 @@ static const struct ctl_table scsi_table[] = {
>>   	  .data		= &scsi_logging_level,
>>   	  .maxlen	= sizeof(scsi_logging_level),
>>   	  .mode		= 0644,
>> -	  .proc_handler	= proc_dointvec },
>> +	  .proc_handler	= proc_dointvec_minmax,
>> +	  .extra1	= SYSCTL_ZERO,
>> +	  .extra2	= SYSCTL_INT_MAX },
> scsi_logging_level is a bitmask and should be unsigned.
>

Hi Martin,

Thank's for your review.

Does `scsi_logging_level` needs the full range of a unsigned 32-bit 
integer ?
As it was using `proc_dointvec`, it was capped to an INT_MAX.

If it effectively need the full range of an unsigned 32-bit integer, the
`proc_handler` could be changed to `proc_douintvec` as suggested by Chuck.

Best regards,

Nicolas

Re: [PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[Joel Granados]

On Tue, Feb 25, 2025 at 11:47:42AM +0100, Nicolas Bouchinet wrote:
> 
> On 2/25/25 02:20, Martin K. Petersen wrote:
> > Hi Nicolas!
> > 
> > > --- a/drivers/scsi/scsi_sysctl.c
> > > +++ b/drivers/scsi/scsi_sysctl.c
> > > @@ -17,7 +17,9 @@ static const struct ctl_table scsi_table[] = {
> > >   	  .data		= &scsi_logging_level,
> > >   	  .maxlen	= sizeof(scsi_logging_level),
> > >   	  .mode		= 0644,
> > > -	  .proc_handler	= proc_dointvec },
> > > +	  .proc_handler	= proc_dointvec_minmax,
> > > +	  .extra1	= SYSCTL_ZERO,
> > > +	  .extra2	= SYSCTL_INT_MAX },
> > scsi_logging_level is a bitmask and should be unsigned.
> > 
> 
> Hi Martin,
> 
> Thank's for your review.
> 
> Does `scsi_logging_level` needs the full range of a unsigned 32-bit integer
> ?
> As it was using `proc_dointvec`, it was capped to an INT_MAX.
> 
> If it effectively need the full range of an unsigned 32-bit integer, the
> `proc_handler` could be changed to `proc_douintvec` as suggested by Chuck.
And as mentioned in another patch in this series:
1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
   silently capped by proc_dointvec_minmax, but it is good to have as it
   adds to the understanding on what the range of the values are.

2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
   prevent ppl trying to assigning negative values to the unsigned integer

Let me know if you take this through the scsi subsystem so I know to
drop it from sysctl 

Best

Reviewed-by: Joel Granados <joel.granados@kernel.org>


> 
> Best regards,
> 
> Nicolas
> 

-- 

Joel Granados

Re: [PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[Martin K. Petersen]

Joel,

> 1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
>    silently capped by proc_dointvec_minmax, but it is good to have as it
>    adds to the understanding on what the range of the values are.
>
> 2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
>    prevent ppl trying to assigning negative values to the unsigned integer
>
> Let me know if you take this through the scsi subsystem so I know to
> drop it from sysctl 

Applied to 6.15/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH v2 4/6] sysctl: Fixes scsi_logging_level bounds

[Joel Granados]

On Mon, Mar 03, 2025 at 09:24:43PM -0500, Martin K. Petersen wrote:
> 
> Joel,
> 
> > 1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
> >    silently capped by proc_dointvec_minmax, but it is good to have as it
> >    adds to the understanding on what the range of the values are.
> >
> > 2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
> >    prevent ppl trying to assigning negative values to the unsigned integer
> >
> > Let me know if you take this through the scsi subsystem so I know to
> > drop it from sysctl 
> 
> Applied to 6.15/scsi-staging, thanks!
Thx!!

@Nicolas: Please take this out of your next version as it is already
going upstream.

Best

> 
> -- 
> Martin K. Petersen	Oracle Linux Engineering

-- 

Joel Granados

[PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 drivers/infiniband/core/iwcm.c | 4 +++-
 drivers/infiniband/core/ucma.c | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
index 7e3a55349e107..f4486cbd8f45a 100644
--- a/drivers/infiniband/core/iwcm.c
+++ b/drivers/infiniband/core/iwcm.c
@@ -109,7 +109,9 @@ static struct ctl_table iwcm_ctl_table[] = {
 		.data		= &default_backlog,
 		.maxlen		= sizeof(default_backlog),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 };
 
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 02f1666f3cbab..6e700b9740331 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -69,7 +69,9 @@ static struct ctl_table ucma_ctl_table[] = {
 		.data		= &max_backlog,
 		.maxlen		= sizeof max_backlog,
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ZERO,
+		.extra2		= SYSCTL_INT_MAX,
 	},
 };
 
-- 
2.48.1

Re: [PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[Leon Romanovsky]

On Mon, Feb 24, 2025 at 10:58:20AM +0100, nicolas.bouchinet@clip-os.org wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
> and SYSCTL_INT_MAX.
> 
> The proc_handler has thus been updated to proc_dointvec_minmax.
> 
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>  drivers/infiniband/core/iwcm.c | 4 +++-
>  drivers/infiniband/core/ucma.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 

Acked-by: Leon Romanovsky <leon@kernel.org>

How do you want to proceed from here? Should I take to RDMA repository?

Thanks

Re: [PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[Joel Granados]

On Mon, Feb 24, 2025 at 03:41:05PM +0200, Leon Romanovsky wrote:
> On Mon, Feb 24, 2025 at 10:58:20AM +0100, nicolas.bouchinet@clip-os.org wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
> > and SYSCTL_INT_MAX.
> > 
> > The proc_handler has thus been updated to proc_dointvec_minmax.
> > 
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > ---
> >  drivers/infiniband/core/iwcm.c | 4 +++-
> >  drivers/infiniband/core/ucma.c | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> > 
> 
> Acked-by: Leon Romanovsky <leon@kernel.org>
> 
> How do you want to proceed from here? Should I take to RDMA repository?
> 
> Thanks
It would be great if we push this through RDMA. Here are a few comments:
1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
   silently capped by proc_dointvec_minmax, but it is good to have as it
   gives understanding on what the spread of the values are.

2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
   prevent ppl trying to assing negative values to the unsigned integers

Please let me know if you will push this through RDMA, so I know to
remove it from sysctl.

Best

Reviewed-by: Joel Granados <joel.granados@kernel.org>

-- 

Joel Granados

Re: [PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[Leon Romanovsky]

On Mon, Mar 03, 2025 at 02:57:29PM +0100, Joel Granados wrote:
> On Mon, Feb 24, 2025 at 03:41:05PM +0200, Leon Romanovsky wrote:
> > On Mon, Feb 24, 2025 at 10:58:20AM +0100, nicolas.bouchinet@clip-os.org wrote:
> > > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > 
> > > Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
> > > and SYSCTL_INT_MAX.
> > > 
> > > The proc_handler has thus been updated to proc_dointvec_minmax.
> > > 
> > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > ---
> > >  drivers/infiniband/core/iwcm.c | 4 +++-
> > >  drivers/infiniband/core/ucma.c | 4 +++-
> > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > 
> > 
> > Acked-by: Leon Romanovsky <leon@kernel.org>
> > 
> > How do you want to proceed from here? Should I take to RDMA repository?
> > 
> > Thanks
> It would be great if we push this through RDMA. Here are a few comments:
> 1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
>    silently capped by proc_dointvec_minmax, but it is good to have as it
>    gives understanding on what the spread of the values are.
> 
> 2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
>    prevent ppl trying to assing negative values to the unsigned integers
> 
> Please let me know if you will push this through RDMA, so I know to
> remove it from sysctl.

Applied to RDMA tree.
https://web.git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=f33cd9b3fd03a791296ab37550ffd26213a90c4e

> 
> Best
> 
> Reviewed-by: Joel Granados <joel.granados@kernel.org>

Thanks

> 
> -- 
> 
> Joel Granados

Re: [PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[Joel Granados]

On Mon, Mar 03, 2025 at 08:53:09PM +0200, Leon Romanovsky wrote:
> On Mon, Mar 03, 2025 at 02:57:29PM +0100, Joel Granados wrote:
> > On Mon, Feb 24, 2025 at 03:41:05PM +0200, Leon Romanovsky wrote:
> > > On Mon, Feb 24, 2025 at 10:58:20AM +0100, nicolas.bouchinet@clip-os.org wrote:
> > > > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > > 
> > > > Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
> > > > and SYSCTL_INT_MAX.
> > > > 
> > > > The proc_handler has thus been updated to proc_dointvec_minmax.
> > > > 
> > > > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > > > ---
> > > >  drivers/infiniband/core/iwcm.c | 4 +++-
> > > >  drivers/infiniband/core/ucma.c | 4 +++-
> > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > > 
> > > 
> > > Acked-by: Leon Romanovsky <leon@kernel.org>
> > > 
> > > How do you want to proceed from here? Should I take to RDMA repository?
> > > 
> > > Thanks
> > It would be great if we push this through RDMA. Here are a few comments:
> > 1. Having the upper bound be SYSCTL_INT_MAX is not necessary (as it is
> >    silently capped by proc_dointvec_minmax, but it is good to have as it
> >    gives understanding on what the spread of the values are.
> > 
> > 2. Having the lower bound capped by SYSCTL_ZERO is needed as it will
> >    prevent ppl trying to assing negative values to the unsigned integers
> > 
> > Please let me know if you will push this through RDMA, so I know to
> > remove it from sysctl.
> 
> Applied to RDMA tree.
> https://web.git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/leon-for-next&id=f33cd9b3fd03a791296ab37550ffd26213a90c4e
Perfect. Thx.

@Nicolas: pls take this one out of your next version as it is
already on its way upstrea.

Best
> 
> > 
> > Best
> > 
> > Reviewed-by: Joel Granados <joel.granados@kernel.org>
> 
> Thanks
> 
> > 
> > -- 
> > 
> > Joel Granados

-- 

Joel Granados

Re: [PATCH v2 5/6] sysctl/infiniband: Fixes infiniband sysctl bounds

[Zhu Yanjun]

在 2025/2/24 10:58, nicolas.bouchinet@clip-os.org 写道:
> From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
>
> Bound infiniband iwcm and ucma sysctl writings between SYSCTL_ZERO
> and SYSCTL_INT_MAX.
>
> The proc_handler has thus been updated to proc_dointvec_minmax.

In this commit, the minimum and maximum are added to the proc_handler.

It seems that this will not introduce any risk.

I am fine with it.

Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>

Thanks,

Zhu Yanjun

>
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> ---
>   drivers/infiniband/core/iwcm.c | 4 +++-
>   drivers/infiniband/core/ucma.c | 4 +++-
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/iwcm.c b/drivers/infiniband/core/iwcm.c
> index 7e3a55349e107..f4486cbd8f45a 100644
> --- a/drivers/infiniband/core/iwcm.c
> +++ b/drivers/infiniband/core/iwcm.c
> @@ -109,7 +109,9 @@ static struct ctl_table iwcm_ctl_table[] = {
>   		.data		= &default_backlog,
>   		.maxlen		= sizeof(default_backlog),
>   		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
> +		.extra2		= SYSCTL_INT_MAX,
>   	},
>   };
>   
> diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
> index 02f1666f3cbab..6e700b9740331 100644
> --- a/drivers/infiniband/core/ucma.c
> +++ b/drivers/infiniband/core/ucma.c
> @@ -69,7 +69,9 @@ static struct ctl_table ucma_ctl_table[] = {
>   		.data		= &max_backlog,
>   		.maxlen		= sizeof max_backlog,
>   		.mode		= 0644,
> -		.proc_handler	= proc_dointvec,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= SYSCTL_ZERO,
> +		.extra2		= SYSCTL_INT_MAX,
>   	},
>   };
>   

-- 
Best Regards,
Yanjun.Zhu

[PATCH v2 6/6] sysctl: Fixes max-user-freq bounds

[nicolas.bouchinet]

From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>

Bound max-user-freq sysctl writings between SYSCTL_ZERO
and SYSCTL_INT_MAX.

The proc_handler has thus been updated to proc_dointvec_minmax.

Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
---
 drivers/char/hpet.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index e110857824fcb..528b43e893d49 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -730,7 +730,9 @@ static const struct ctl_table hpet_table[] = {
 	 .data = &hpet_max_freq,
 	 .maxlen = sizeof(int),
 	 .mode = 0644,
-	 .proc_handler = proc_dointvec,
+	 .proc_handler = proc_dointvec_minmax,
+	 .extra1 = SYSCTL_ZERO,
+	 .extra2 = SYSCTL_INT_MAX,
 	 },
 };
 
-- 
2.48.1

Re: [PATCH v2 0/6] Fixes multiple sysctl bound checks

[Joel Granados]

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
> 

-- 

Joel Granados

Re: (subset) [PATCH v2 0/6] Fixes multiple sysctl bound checks

[Martin K. Petersen]

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

-- 
Martin K. Petersen	Oracle Linux Engineering