[PATCH] initramfs: Fix a missing-check bug in init/initramfs.c

[PATCH] initramfs: Fix a missing-check bug in init/initramfs.c

[Gen Zhang]

In dir_add(), de and de->name are allocated by kmalloc() and kstrdup().
And de->name is dereferenced in the following codes. However, memory
allocation functions such as kmalloc() and kstrdup() may fail.
Dereferencing this de->name null pointer may cause the kernel go wrong.
Thus we should check this allocation.
Further, if kstrdup() returns NULL, we should free de and panic().

Signed-off-by: Gen Zhang <blackgod016574@gmail.com>

---
diff --git a/init/initramfs.c b/init/initramfs.c
index 178130f..dc8063f 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -125,6 +125,10 @@ static void __init dir_add(const char *name, time64_t mtime)
 		panic("can't allocate dir_entry buffer");
 	INIT_LIST_HEAD(&de->list);
 	de->name = kstrdup(name, GFP_KERNEL);
+	if (!de->name) {
+		kfree(de);
+		panic("can't allocate dir_entry name buffer");
+	}
 	de->mtime = mtime;
 	list_add(&de->list, &dir_list);
 }

Re: [PATCH] initramfs: Fix a missing-check bug in init/initramfs.c

[Gen Zhang]

On Wed, May 22, 2019 at 10:00:37AM +0800, Li Zhijian wrote:
> 
> On 5/22/19 09:04, Gen Zhang wrote:
> >In dir_add(), de and de->name are allocated by kmalloc() and kstrdup().
> >And de->name is dereferenced in the following codes. However, memory
> >allocation functions such as kmalloc() and kstrdup() may fail.
> >Dereferencing this de->name null pointer may cause the kernel go wrong.
> >Thus we should check this allocation.
> >Further, if kstrdup() returns NULL, we should free de and panic().
> >
> >Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> >
> >---
> >diff --git a/init/initramfs.c b/init/initramfs.c
> >index 178130f..dc8063f 100644
> >--- a/init/initramfs.c
> >+++ b/init/initramfs.c
> >@@ -125,6 +125,10 @@ static void __init dir_add(const char *name, time64_t mtime)
> >  		panic("can't allocate dir_entry buffer");
> >  	INIT_LIST_HEAD(&de->list);
> >  	de->name = kstrdup(name, GFP_KERNEL);
> >+	if (!de->name) {
> >+		kfree(de);
> >+		panic("can't allocate dir_entry name buffer");
> >+	}
> 
> Looks good
> 
> but the following place should be considered as well i think
> 342                                 vcollected = kstrdup(collected, GFP_KERNEL);
> 343                                 state = CopyFile;
> 
> 
> Thanks
> Zhijian
Thanks for your comments, Zhijian!
I thinks you are correct that vcollected should also be checked.
I will work on this patch and resubmit it.
Thank
Gen

Re: [PATCH v2] initramfs: Fix a missing-check bug in init/initramfs.c

[Gen Zhang]

On Wed, May 22, 2019 at 10:00:37AM +0800, Li Zhijian wrote:
> Looks good
> 
> but the following place should be considered as well i think
> 342                                 vcollected = kstrdup(collected, GFP_KERNEL);
> 343                                 state = CopyFile;
> 
> 
> Thanks
> Zhijian
In dir_add() and do_name(), de->name and vcollected are allocated by
kstrdup(). And de->name and vcollected are dereferenced in the following
codes. However, memory allocation functions such as kstrdup() may fail. 
Dereferencing this null pointer may cause the kernel go wrong. Thus we
should check these two kstrdup() operations.
Further, if kstrdup() returns NULL, we should free de in dir_add().

Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/init/initramfs.c b/init/initramfs.c
index 178130f..1421488 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -125,6 +125,10 @@ static void __init dir_add(const char *name, time64_t mtime)
 		panic("can't allocate dir_entry buffer");
 	INIT_LIST_HEAD(&de->list);
 	de->name = kstrdup(name, GFP_KERNEL);
+	if (!de->name) {
+		kfree(de);
+		panic("can't allocate dir_entry name buffer");
+	}
 	de->mtime = mtime;
 	list_add(&de->list, &dir_list);
 }
@@ -340,6 +344,10 @@ static int __init do_name(void)
 				if (body_len)
 					ksys_ftruncate(wfd, body_len);
 				vcollected = kstrdup(collected, GFP_KERNEL);
+				if (!vcollected) {
+					panic("can't allocate vcollected buffer");
+					return 0;
+				}
 				state = CopyFile;
 			}
 		}

[PATCH v2] initramfs: Fix a missing-check bug in init/initramfs.c

[Gen Zhang]

In dir_add() and do_name(), de->name and vcollected are allocated by
kstrdup(). And de->name and vcollected are dereferenced in the following
codes. However, memory allocation functions such as kstrdup() may fail. 
Dereferencing this null pointer may cause the kernel go wrong. Thus we
should check these two kstrdup() operations.
Further, if kstrdup() returns NULL, we should free de in dir_add().

Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
---
diff --git a/init/initramfs.c b/init/initramfs.c
index 178130f..1421488 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -125,6 +125,10 @@ static void __init dir_add(const char *name, time64_t mtime)
 		panic("can't allocate dir_entry buffer");
 	INIT_LIST_HEAD(&de->list);
 	de->name = kstrdup(name, GFP_KERNEL);
+	if (!de->name) {
+		kfree(de);
+		panic("can't allocate dir_entry name buffer");
+	}
 	de->mtime = mtime;
 	list_add(&de->list, &dir_list);
 }
@@ -340,6 +344,10 @@ static int __init do_name(void)
 				if (body_len)
 					ksys_ftruncate(wfd, body_len);
 				vcollected = kstrdup(collected, GFP_KERNEL);
+				if (!vcollected) {
+					panic("can't allocate vcollected buffer");
+					return 0;
+				}
 				state = CopyFile;
 			}
 		}