2019-12-10 02:48:57

by zhanglin

Subject: [PATCH] initramfs: forcing panic when kstrdup failed

preventing further undefined behaviour when kstrdup failed.

Signed-off-by: zhanglin <[email protected]>
---
init/initramfs.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/init/initramfs.c b/init/initramfs.c
index fca899622937..39a4fba48cc7 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -125,6 +125,8 @@ 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)
+ panic("can't allocate dir_entry.name buffer");
de->mtime = mtime;
list_add(&de->list, &dir_list);
}
@@ -340,6 +342,8 @@ static int __init do_name(void)
if (body_len)
ksys_ftruncate(wfd, body_len);
vcollected = kstrdup(collected, GFP_KERNEL);
+ if (!vcollected)
+ panic("can not allocate vcollected buffer.");
state = CopyFile;
}
}
--
2.17.1

2019-12-10 07:09:41

by David Engraf

Subject: Re: [PATCH] initramfs: forcing panic when kstrdup failed

On 10.12.19 at 03:48, zhanglin wrote:
> preventing further undefined behaviour when kstrdup failed.
>
> Signed-off-by: zhanglin <[email protected]>
> ---
> init/initramfs.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/init/initramfs.c b/init/initramfs.c
> index fca899622937..39a4fba48cc7 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -125,6 +125,8 @@ 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)
> + panic("can't allocate dir_entry.name buffer");
> de->mtime = mtime;
> list_add(&de->list, &dir_list);
> }
> @@ -340,6 +342,8 @@ static int __init do_name(void)
> if (body_len)
> ksys_ftruncate(wfd, body_len);
> vcollected = kstrdup(collected, GFP_KERNEL);
> + if (!vcollected)
> + panic("can not allocate vcollected buffer.");

I would change the message to have the same naming as the other out of
memory messages:

panic("can't allocate vcollected buffer");

Best regards
- David

> state = CopyFile;
> }
> }
>

2019-12-10 08:17:22

by Geert Uytterhoeven

Subject: Re: [PATCH] initramfs: forcing panic when kstrdup failed

On Tue, Dec 10, 2019 at 3:47 AM zhanglin <[email protected]> wrote:
> preventing further undefined behaviour when kstrdup failed.
>
> Signed-off-by: zhanglin <[email protected]>

Thanks for your patch!

> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -125,6 +125,8 @@ 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)
> + panic("can't allocate dir_entry.name buffer");
> de->mtime = mtime;
> list_add(&de->list, &dir_list);
> }
> @@ -340,6 +342,8 @@ static int __init do_name(void)
> if (body_len)
> ksys_ftruncate(wfd, body_len);
> vcollected = kstrdup(collected, GFP_KERNEL);
> + if (!vcollected)
> + panic("can not allocate vcollected buffer.");
> state = CopyFile;
> }
> }

Do we really need to add more messages for out-of-memory conditions?
The trend is to remove the printing of those messages, as the memory
allocation subsystem will have printed a backtrace already anyway.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-12-11 00:54:17

by Andrew Morton

Subject: Re: [PATCH] initramfs: forcing panic when kstrdup failed

On Tue, 10 Dec 2019 09:15:54 +0100 Geert Uytterhoeven <[email protected]> wrote:

> > --- a/init/initramfs.c
> > +++ b/init/initramfs.c
> > @@ -125,6 +125,8 @@ 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)
> > + panic("can't allocate dir_entry.name buffer");
> > de->mtime = mtime;
> > list_add(&de->list, &dir_list);
> > }
> > @@ -340,6 +342,8 @@ static int __init do_name(void)
> > if (body_len)
> > ksys_ftruncate(wfd, body_len);
> > vcollected = kstrdup(collected, GFP_KERNEL);
> > + if (!vcollected)
> > + panic("can not allocate vcollected buffer.");
> > state = CopyFile;
> > }
> > }
>
> Do we really need to add more messages for out-of-memory conditions?
> The trend is to remove the printing of those messages, as the memory
> allocation subsystem will have printed a backtrace already anyway.

Yup. And we traditionally assume that memory allocations won't fail at
init time anyway. The reasoning being that the system is so enormously
messed up that the problem is both unrecoverable and very obvious.