On Thu, Oct 11, 2012 at 10:04:28PM +0800, Jeremy Kerr wrote: > Hi Andy, > > >@@ -969,16 +970,18 @@ > > return -ENOMEM; > > > > 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; > > > >+ inode = NULL; > >+ > > len = utf16_strlen(entry->var.VariableName); > > > > /* GUID plus trailing NULL */ > > name = kmalloc(len + 38, GFP_ATOMIC); > >+ if (!name) > >+ goto fail; > > > > for (i = 0; i < len; i++) > > name[i] = entry->var.VariableName[i] & 0xFF; > >@@ -991,7 +994,13 @@ > > > > inode = efivarfs_get_inode(efivarfs_sb, root->d_inode, > > S_IFREG | 0644, 0); > >+ if (!inode) > >+ goto fail_name; > >+ > > dentry = d_alloc_name(root, name); > >+ if (!dentry) > >+ goto fail_inode; > >+ > > /* copied by the above to local storage in the dentry. */ > > kfree(name); > > If we break out of the loop on the second (and onwards) iteration, > won't we still have the other inodes and dentries remaining > allocated? As we calling this from the mount_single() wrapper: return mount_single(fs_type, flags, data, efivarfs_fill_super); which does this: struct dentry *mount_single(struct file_system_type *fs_type, int flags, void *data, int (*fill_super)(struct super_block *, void *, int)) { [...] error = fill_super(s, data, flags & MS_SILENT ? 1 : 0); if (error) { deactivate_locked_super(s); return ERR_PTR(error); } [...] I am expecting us to get called back via deactivate_locked_super(), which calls sb->kill_sb() which is: static void efivarfs_kill_sb(struct super_block *sb) { kill_litter_super(sb); efivarfs_sb = NULL; } Which I believe will clean them up. -apw -- 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