Re: [PATCH v2 3/5] firmware: check for possible file truncation early

[Luis R. Rodriguez]

On Tue, May 12, 2015 at 01:35:59PM -0700, Linus Torvalds wrote:
> On Tue, May 12, 2015 at 11:30 AM, Luis R. Rodriguez
> <mcgrof@xxxxxxxxxxxxxxxx> wrote:
> >
> > Instead of waiting until the last second to fail
> > on a request_firmware*() calls due to filename
> > truncation we can do an early check upon boot
> > and immediatley avoid any possible issues upfront.
> 
> Why? This looks stupid. Why add this special case, when normal path
> lookup results in the proper errors

It seemed silly to proceed late if we can catch the possible name errors
early. It does indeed have the cost of all that early cruft code.

> And if invalid pathnames are so frequent that this special case is
> somehow worth it, we should fix whoever generates that crap, instead
> of adding this insane special case.

OK, I'm all for ignoring non-upstream drivers.

> And if we don't handle the errors from normal path lookup properly,
> then we should fix *that*.

That was done on patch 2, originally I was going for a simple early
check which can be put on all API calls prior to doing anything too
intrusive:

+static int sysdata_validate_filename(const char *name)
+{
+       if (!name)
+               return -EINVAL;
+       /* POSIX.1 2.4: an empty pathname is invalid, match other checks */
+       if (name[0] == '\0')
+               return -ENOENT;
+}

Since the truncation was possible too though it seemed worthy to add given
that quite a few callers can end up re-using the same code.

> In other words, I see absolutely no reason for this patch. Regardless
> of the reason for it, it seems entirely broken.

OK I'll get rid of all these early checks.

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/