From: Ladislav Michl <ladis@linux-mips.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/1] ubifs: avoid possible NULL dereference
Date: Wed, 22 Nov 2017 09:25:31 +0100 [thread overview]
Message-ID: <20171122082531.GA15601@lenoch> (raw)
In-Reply-To: <20171122080936.32ECB12051D@gemini.denx.de>
On Wed, Nov 22, 2017 at 09:09:36AM +0100, Wolfgang Denk wrote:
> Dear Ladislav,
>
> In message <20171121212222.ryicwv6tyh5rye2e@lenoch> you wrote:
> > > >
> > > > diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
> > > > index 4465523d5f..313dee0579 100644
> > > > --- a/fs/ubifs/ubifs.c
> > > > +++ b/fs/ubifs/ubifs.c
> > > > @@ -403,8 +403,7 @@ static int ubifs_finddir(struct super_block *sb, char *dirname,
> > > > dir = kzalloc(sizeof(struct inode), 0);
> > > > if (!file || !dentry || !dir) {
> > > > printf("%s: Error, no memory for malloc!\n", __func__);
> > > > - err = -ENOMEM;
> > > > - goto out;
> > > > + goto out_nomem;
> > > > }
> ...
> > > Should you not keep the "err = -ENOMEM;" setting? Otherwise there
> > > is no indivcation that an error happened.
> >
> > It is not obvious from the patch, but value of err is later discarded.
> > It serves sole purpose of printing debug notice.
>
> So apparently we have a number of places in U-Boot where fatal
> errors (running out of memory) are just ignored and we continue as
> if nothing happened?
While I have to admit this code is not an example of clean coding,
it prints notice when trying to manipulate with file.
fs/ubifs/ubifs.c as whole needs to be revisited, above patch just
caused shit hitting the fan.
> THis is short-sighted at best. One day Pump Six will fail.
>
> This is giving me the creepes.
I was just pointing to the fact, that above mentioned patch does not
make it any worse. Btw, initial commit is even more amazing:
+ if (file)
+ free(file);
+ if (dentry)
+ free(dentry);
+ if (dir)
+ free(dir);
+
+ if (file->private_data)
+ kfree(file->private_data);
+ file->private_data = NULL;
+ file->f_pos = 2;
+ return 0;
.oO(I guess it is safe not pointing where above snippet is comming from,
but will review whole file as I'm going to use it for new board I have to
suport)
Best regards,
ladis
next prev parent reply other threads:[~2017-11-22 8:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-21 18:45 [U-Boot] [PATCH 1/1] ubifs: avoid possible NULL dereference Heinrich Schuchardt
2017-11-21 20:23 ` Ladislav Michl
2017-11-21 21:29 ` Heinrich Schuchardt
2017-11-21 21:55 ` Ladislav Michl
2017-11-21 21:16 ` Wolfgang Denk
2017-11-21 21:22 ` Ladislav Michl
2017-11-22 8:09 ` Wolfgang Denk
2017-11-22 8:25 ` Ladislav Michl [this message]
2017-11-24 5:59 ` Heiko Schocher
2017-11-21 21:28 ` Heinrich Schuchardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171122082531.GA15601@lenoch \
--to=ladis@linux-mips.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox