[PATCH 1/4] efi: Add support for a UEFI variable filesystem

[Tetsuo Handa]

Andy Whitcroft wrote:
> +static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
> +		size_t count, loff_t *ppos)
> +{
> +	struct efivar_entry *var = file->private_data;
> +	struct efivars *efivars = var->efivars;
> +	efi_status_t status;
> +	unsigned long datasize = 0;
> +	u32 attributes;
> +	void *data;
> +	ssize_t size;
> +
> +	status = efivars->ops->get_variable(var->var.VariableName,
> +					    &var->var.VendorGuid,
> +					    &attributes, &datasize, NULL);
> +
> +	if (status != EFI_BUFFER_TOO_SMALL)
> +		return 0;
> +
> +	data = kmalloc(datasize + 4, GFP_KERNEL);
> +
> +	if (!data)
> +		return 0;
> +
> +	status = efivars->ops->get_variable(var->var.VariableName,
> +					    &var->var.VendorGuid,
> +					    &attributes, &datasize,
> +					    (data + 4));
> +
> +	if (status != EFI_SUCCESS)

Why no kfree(data) before return?

> +		return 0;
> +
> +	memcpy(data, &attributes, 4);
> +	size = simple_read_from_buffer(userbuf, count, ppos,
> +					data, datasize + 4);
> +	kfree(data);
> +
> +	return size;
> +}

> +int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
> +{
> +	struct inode *inode = NULL;
> +	struct dentry *root;
> +	struct efivar_entry *entry, *n;
> +	struct efivars *efivars = &__efivars;
> +	int err;
> +
> +	efivarfs_sb = sb;
> +
> +	sb->s_maxbytes          = MAX_LFS_FILESIZE;
> +	sb->s_blocksize         = PAGE_CACHE_SIZE;
> +	sb->s_blocksize_bits    = PAGE_CACHE_SHIFT;
> +	sb->s_magic             = PSTOREFS_MAGIC;
> +	sb->s_op                = &efivarfs_ops;
> +	sb->s_time_gran         = 1;
> +
> +	inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0);
> +	if (!inode) {
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +	inode->i_op = &efivarfs_dir_inode_operations;
> +
> +	root = d_make_root(inode);
> +	sb->s_root = root;
> +	if (!root) {
> +		err = -ENOMEM;
> +		goto fail;
> +	}
> +
> +	list_for_each_entry_safe(entry, n, &efivars->list, list) {
> +		struct inode *inode;
> +		struct dentry *dentry, *root = efivarfs_sb->s_root;
> +		char *name;
> +		unsigned long size = 0;
> +		int len, i;
> +
> +		len = utf16_strlen(entry->var.VariableName);
> +
> +		/* GUID plus trailing NULL */
> +		name = kmalloc(len + 38, GFP_ATOMIC);

Why no name != NULL check?

> +
> +		for (i = 0; i < len; i++)
> +			name[i] = entry->var.VariableName[i] & 0xFF;
> +
> +		name[len] = '-';
> +
> +		efi_guid_unparse(&entry->var.VendorGuid, name + len + 1);
> +
> +		name[len+GUID_LEN] = '\0';
> +
> +		inode = efivarfs_get_inode(efivarfs_sb, root->d_inode,
> +					  S_IFREG | 0644, 0);

Why no inode != NULL check?

> +		dentry = d_alloc_name(root, name);

Why no dentry != NULL check?

> +
> +		efivars->ops->get_variable(entry->var.VariableName,
> +					   &entry->var.VendorGuid,
> +					   &entry->var.Attributes,
> +					   &size,
> +					   NULL);
> +
> +		mutex_lock(&inode->i_mutex);
> +		inode->i_private = entry;
> +		i_size_write(inode, size+4);
> +		mutex_unlock(&inode->i_mutex);
> +		d_add(dentry, inode);
> +	}
> +
> +	return 0;
> +fail:
> +	iput(inode);
> +	return err;
> +}