[PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Srikanth K H]

If the make directory for "sys" interface fail's then its
dereferenced without even checking for its validity which
will lead to crash, hence added preventive code to check
for NULL and accordingly dereference.

Signed-off-by: Srikanth K H <srikanth.h@samsung.com>
---
 fs/proc/proc_sysctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 89921a0..320884b 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -1692,6 +1692,8 @@ int __init proc_sys_init(void)
 	struct proc_dir_entry *proc_sys_root;
 
 	proc_sys_root = proc_mkdir("sys", NULL);
+	if (!proc_sys_root)
+		return -ENOMEM;
 	proc_sys_root->proc_iops = &proc_sys_dir_operations;
 	proc_sys_root->proc_fops = &proc_sys_dir_file_operations;
 	proc_sys_root->nlink = 0;
-- 
1.9.1

Re: [PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Alexey Dobriyan]

On Thu, Aug 16, 2018 at 03:04:13PM +0530, Srikanth K H wrote:
> If the make directory for "sys" interface fail's then its
> dereferenced without even checking for its validity which
> will lead to crash, hence added preventive code to check
> for NULL and accordingly dereference.

> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -1692,6 +1692,8 @@ int __init proc_sys_init(void)
>  	struct proc_dir_entry *proc_sys_root;
>  
>  	proc_sys_root = proc_mkdir("sys", NULL);
> +	if (!proc_sys_root)
> +		return -ENOMEM;
>  	proc_sys_root->proc_iops = &proc_sys_dir_operations;
>  	proc_sys_root->proc_fops = &proc_sys_dir_file_operations;
>  	proc_sys_root->nlink = 0;

It is fine to crash because /proc is not modular.

RE: Re: [PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Srikanth Korangala Hari]

> It is fine to crash because /proc is not modular.

Dear Alexey, this was theoretical solution. If you feel this should crash instead if the call fail's then ignore the patch.

Regards,
Srikanth
 

Re: [PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Luis Chamberlain]

On Thu, Aug 16, 2018 at 03:04:13PM +0530, Srikanth K H wrote:
> If the make directory for "sys" interface fail's then its
> dereferenced without even checking for its validity which
> will lead to crash, hence added preventive code to check
> for NULL and accordingly dereference.
> 
> Signed-off-by: Srikanth K H <srikanth.h@samsung.com>

Thanks for the patch! Do you have a reproducer or is this theoretical?
This will affect if it should go to stable or not.

  Luis

RE: Re: [PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Srikanth Korangala Hari]

> Thanks for the patch! Do you have a reproducer or is this theoretical?
> This will affect if it should go to stable or not.

Dear Luis, this is theoretical as I observed in most of the call's to api - "proc_mkdir" the NULL check is being done. Hence I thought of adding one here.

Regards,
Srikanth

Re: Re: [PATCH 1/1] Preventive patch in the proc file-system to handle NULL check.

[Al Viro]

On Fri, Aug 17, 2018 at 08:51:42AM +0530, Srikanth Korangala Hari wrote:
> > Thanks for the patch! Do you have a reproducer or is this theoretical?
> > This will affect if it should go to stable or not.
> 
> Dear Luis, this is theoretical as I observed in most of the call's to api - "proc_mkdir" the NULL check is being done. Hence I thought of adding one here.

Realistically, if you get allocation failures that early in the boot,
oops is the least of your problems - it won't get through mounting
the root or lauching init (or unpacking initramfs, etc.)

Sure, might as well check it there - nothing wrong with that, but do
keep in mind that
	* it's very certain to end up in panic() very shortly afterwards,
no matter what
	* the odds of exhausting memory (and that would be extremely
low-memory setup) precisely at that point (i.e. even getting to
proc_root_init()) are not high.

Might be interesting to try lower and lower mem=... values passed to the
kernel in attempt to step into this one; I wouldn't put large odds on
being able to hit precisely that place, but it would be educational anyway.