When mounting ubifs e.g. through command 'ubifsmount' one global static superblock 'ubifs_sb' is used _and_ the requested volume is opened (like in Linux). The pointer returned by 'ubifs_open_volume()' is stored in that superblock struct and freed later on cmd 'ubifsumount' or another call to 'ubifsmount' with a different volume, through ubifs_umount() and ubi_close_volume().
In ubifs_ls(), ubifs_exists(), ubifs_size(), and ubifs_read() the volume was opened again, which is technically no problem with regard to refcounting, but here the still valid pointer in sb was overwritten, leading to a memory leak. Even worse, when using one of those functions and calling ubifsumount later, ubi_close_volume() was called again but now on an already freed pointer, leading to a double free. This actually crashed with different invalid memory accesses on a board using the old distro boot and a rather long script handling RAUC updates.
Example:
> ubi part UBI > ubifsmount ubi0:boot > test -e ubi ubi0:boot /boot.scr.uimg > ubifsumount
The ubifs specific commands 'ubifsls' and 'ubifsload' check for a mounted volume by themselves, for the generic fs variants 'ls', 'load', (and 'size', and 'test -e') this is covered by special ubifs handling in fs_set_blk_dev() and deeper down blk_get_device_part_str() then. So for ubifs_ls(), ubifs_exists(), ubifs_size(), and ubifs_read() we can be sure the volume is opened and the necessary struct pointer in sb is valid, so it is not needed to open volume again.
Fixes: 9eefe2a2b37 ("UBIFS: Implement read-only UBIFS support in U-Boot") Fixes: 29cc5bcadfc ("ubifs: Add functions for generic fs use") Signed-off-by: Alexander Dahl ada@thorsis.com --- fs/ubifs/ubifs.c | 12 ------------ 1 file changed, 12 deletions(-)
diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 75de01e95f7..58b3f3659c1 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -566,7 +566,6 @@ int ubifs_set_blk_dev(struct blk_desc *rbdd, struct disk_partition *info)
int ubifs_ls(const char *filename) { - struct ubifs_info *c = ubifs_sb->s_fs_info; struct file *file; struct dentry *dentry; struct inode *dir; @@ -574,7 +573,6 @@ int ubifs_ls(const char *filename) unsigned long inum; int ret = 0;
- c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY); inum = ubifs_findfile(ubifs_sb, (char *)filename); if (!inum) { ret = -1; @@ -608,31 +606,24 @@ out_mem: free(dir);
out: - ubi_close_volume(c->ubi); return ret; }
int ubifs_exists(const char *filename) { - struct ubifs_info *c = ubifs_sb->s_fs_info; unsigned long inum;
- c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY); inum = ubifs_findfile(ubifs_sb, (char *)filename); - ubi_close_volume(c->ubi);
return inum != 0; }
int ubifs_size(const char *filename, loff_t *size) { - struct ubifs_info *c = ubifs_sb->s_fs_info; unsigned long inum; struct inode *inode; int err = 0;
- c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY); - inum = ubifs_findfile(ubifs_sb, (char *)filename); if (!inum) { err = -1; @@ -650,7 +641,6 @@ int ubifs_size(const char *filename, loff_t *size)
ubifs_iput(inode); out: - ubi_close_volume(c->ubi); return err; }
@@ -845,7 +835,6 @@ int ubifs_read(const char *filename, void *buf, loff_t offset, return -1; }
- c->ubi = ubi_open_volume(c->vi.ubi_num, c->vi.vol_id, UBI_READONLY); /* ubifs_findfile will resolve symlinks, so we know that we get * the real file here */ inum = ubifs_findfile(ubifs_sb, (char *)filename); @@ -909,7 +898,6 @@ put_inode: ubifs_iput(inode);
out: - ubi_close_volume(c->ubi); return err; }