| Message ID | 644f44ad-7e2b-4a1a-bbd7-ccc79d479242@web.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | pstore/ram: Return directly after a failed kasprintf() call in ramoops_init_prz() | expand |
Hi Markus, Thanks for your patch. On 2024/1/18 04:24, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Wed, 17 Jan 2024 21:09:22 +0100 > > The result from a call of the function “kasprintf” was passed to > a subsequent function call without checking for a null pointer before > (according to a memory allocation failure). > This issue was detected by using the Coccinelle software. > > Thus return directly after a failed kasprintf() call. > > Fixes: 1227daa43bce1 ("pstore/ram: Clarify resource reservation labels") > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > fs/pstore/ram.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 88b34fdbf759..1a673a4af17c 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -595,6 +595,9 @@ static int ramoops_init_prz(const char *name, > } > > label = kasprintf(GFP_KERNEL, "ramoops:%s", name); > + if (!label) > + return -ENOMEM; > + This part looks good to me. Commit 1227daa43bce1 ("pstore/ram: Clarify resource reservation labels") introduce another two more kasprintf in the ramoops_init_przs. Could you fix it together? > *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, > cxt->memtype, PRZ_FLAG_ZAP_OLD, label); > kfree(label); > -- > 2.43.0 >
On Wed, Jan 17, 2024 at 09:24:12PM +0100, Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Wed, 17 Jan 2024 21:09:22 +0100 > > The result from a call of the function “kasprintf” was passed to > a subsequent function call without checking for a null pointer before > (according to a memory allocation failure). > This issue was detected by using the Coccinelle software. > > Thus return directly after a failed kasprintf() call. > > Fixes: 1227daa43bce1 ("pstore/ram: Clarify resource reservation labels") > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > fs/pstore/ram.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 88b34fdbf759..1a673a4af17c 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -595,6 +595,9 @@ static int ramoops_init_prz(const char *name, > } > > label = kasprintf(GFP_KERNEL, "ramoops:%s", name); > + if (!label) > + return -ENOMEM; > + > *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, > cxt->memtype, PRZ_FLAG_ZAP_OLD, label); > kfree(label); This patch is fine as a clean up, but I think it's useful to say that if you pass a NULL label to persistent_ram_new() then it will return an error. It won't crash. So this patch is a nice cleanup but it's not a bug fix. regards, dan carpenter
>> The result from a call of the function “kasprintf” was passed to >> a subsequent function call without checking for a null pointer before >> (according to a memory allocation failure). >> This issue was detected by using the Coccinelle software. … >> +++ b/fs/pstore/ram.c >> @@ -595,6 +595,9 @@ static int ramoops_init_prz(const char *name, >> } >> >> label = kasprintf(GFP_KERNEL, "ramoops:%s", name); >> + if (!label) >> + return -ENOMEM; >> + >> *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, >> cxt->memtype, PRZ_FLAG_ZAP_OLD, label); >> kfree(label); > > This patch is fine as a clean up, but I think it's useful to say that > if you pass a NULL label to persistent_ram_new() then it will return > an error. … Will it become helpful to annotate the corresponding function input parameter for null pointer tolerance anyhow? Regards, Markus
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 88b34fdbf759..1a673a4af17c 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -595,6 +595,9 @@ static int ramoops_init_prz(const char *name, } label = kasprintf(GFP_KERNEL, "ramoops:%s", name); + if (!label) + return -ENOMEM; + *prz = persistent_ram_new(*paddr, sz, sig, &cxt->ecc_info, cxt->memtype, PRZ_FLAG_ZAP_OLD, label); kfree(label);