On Mon, Feb 11, 2013 at 03:22:21PM +0000, Al Viro wrote:
> > +static int efivarfs_d_compare(const struct dentry *parent, const struct inode *pinode,
> > + const struct dentry *dentry, const struct inode *inode,
> > + unsigned int len, const char *str,
> > + const struct qstr *name)
> > +{
> > + const char *q;
> > + int guid;
> > +
> > + /*
> > + * If the string we're being asked to compare doesn't match
> > + * the expected format return "no match".
> > + */
> > + if (!efivarfs_valid_name(str, len))
> > + return 1;
> > + if (!(q = strchr(name->name, '-')))
> > + return 1;
>
> No. Why check that again, when we'd already called ->d_hash() on the
> incoming name *and* candidate dentry? And buggered off on any potential
> errors.
>
> > +
> > + /* Find part 1, the variable name. */
> > + guid = q - (const char *)name->name;
>
> No need to do strchr() for that - you know that name passes
> efivarfs_valid_name(), so you know how far from the end will GUID part begin.
>
> > + /* Case-sensitive compare for the variable name */
> > + if (memcmp(str, name->name, guid))
> > + return 1;
... and by the way, you need to compare lengths first, or that memcmp()
risks running out of mapped page. Sure, it's NUL-terminated, but memcmp()
is *not* required to compare left-to-right; it's arch-dependent and the
very first memory access have every right to be at str + guid - 1.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
