Re: [PATCH 2/2 v2] efivarfs: guid part of filenames are case-insensitive

[Al Viro <viro-3bDd1+5oDREiFSDQTTA3OLVCufUGDwFn@public.gmane.org>]

On Tue, Feb 12, 2013 at 12:39:34PM +0000, Matt Fleming wrote:

> +static struct dentry *efivarfs_alloc_dentry(struct dentry *parent, char *name)
> +{
> +	struct qstr q;
> +
> +	q.name = name;
> +	q.len = strlen(name);
> +
> +	if (efivarfs_d_hash(NULL, NULL, &q))
> +		return NULL;
> +
> +	return d_alloc(parent, &q);
> +}

> @@ -1098,7 +1177,7 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
>  		if (!inode)
>  			goto fail_name;
>  
> -		dentry = d_alloc_name(root, name);
> +		dentry = efivarfs_alloc_dentry(root, name);
>  		if (!dentry)
>  			goto fail_inode;

Umm...  That name has just been built by efivarfs_fill_super() itself, and
AFAICS there's no way for its GUID part to be _not_ lowercase
hex and with proper locations of dashes.  So
	a) hash value will be exactly full_name_hash(name), unless
efivarfs_valid_name() manages to fail.
	b) efivarfs_valid_name() is a serious overkill here - the only things
that might go wrong are length of and dashes in entry->var.VariableName.
Both are trivially checked while we are constructing name (before we do
inode allocation, etc.) and IMO they would be better off there.

IOW, I think this part of the patch is better handled by doing those two
checks several lines before d_alloc_name() and not bothering with
efivarfs_alloc_dentry() at all.  I.e.
		if (len + 1 + GUID_LEN > NAME_MAX)
			goto fail;

		name = kmalloc(len + 1 + GUID_LEN + 1, GFP_ATOMIC);
		if (!name)
			goto fail;

		for (i = 0; i < len; i++) {
			name[i] = entry->var.VariableName[i] & 0xFF;
			if (name[i] == '-')
				goto fail_name;
		}
and then as in the current efivarfs_fill_super().