| Message ID | 1534412053-22457-1-git-send-email-srikanth.h@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] Preventive patch in the proc file-system to handle NULL check. | expand |
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.
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
> 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
> 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
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.
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;
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(+)