UBIFS: fix incorrect unlocking handling
Subsystems (2)
PATCH 1/1 UBIFS: fix incorrect unlocking handling
When ubifs_init_security() fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. There are four such places that were introduce by the last commit.
Signed-off-by Taesoo Kim <[email protected]>
--- fs/ubifs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
fs/ubifs/dir.c +7 -4
@@ -272,7 +272,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
@@ -292,6 +292,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -732,7 +733,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex);insert_inode_hash (inode);
@@ -757,6 +758,7 @@ out_cancel: dir_ui->ui_size = dir->i_size;drop_nlink (dir);mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -816,7 +818,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
@@ -836,6 +838,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -896,7 +899,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
--
2.3.3
Quoted 3 lines · Taesoo Kim
When
ubifs_init_security()fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. There are four such places that were introduce by the last commit.
"The last commit" is not very descriptive. Are you speaking of this commit?
commit d7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53
Author: Subodh Nijsure <[email protected]> Date: Fri Oct 31 13:50:30 2014 -0500
UBIFS: Add security.* XATTR support for the UBIFS
CC'ing authors/reviewers.
Brian
Quoted 72 lines
Signed-off-by Taesoo Kim <[email protected]>--- fs/ubifs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)fs/ubifs/dir.c +7
-5@@ -272,7 +272,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -292,6 +292,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -732,7 +733,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex);insert_inode_hash (inode);@@ -757,6 +758,7 @@ out_cancel: dir_ui->ui_size = dir->i_size;drop_nlink (dir);mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -816,7 +818,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -836,6 +838,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -896,7 +899,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change; --2.3.3
Yes. The last commit that modifies 'ubifs/dir.c' (sorry for missing the context). All error handling routines introduced by 'd7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53' 1) incorrectly 'unlock' and
2) incorrectly restore 'i_size'.
Thanks,
Taesoo
Quoted 90 lines · Brian Norris
Quoted 3 lines · Taesoo Kim
When
ubifs_init_security()fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. There are four such places that were introduce by the last commit."The last commit" is not very descriptive. Are you speaking of this commit?
commit
d7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53Author: Subodh Nijsure <[email protected]> Date: Fri Oct 31 13:50:30 2014 -0500UBIFS: Add security.* XATTR support for the UBIFS
CC'ing authors/reviewers.
Brian
Quoted 72 lines
Signed-off-by Taesoo Kim <[email protected]>--- fs/ubifs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)fs/ubifs/dir.c +7
-5@@ -272,7 +272,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -292,6 +292,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -732,7 +733,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex);insert_inode_hash (inode);@@ -757,6 +758,7 @@ out_cancel: dir_ui->ui_size = dir->i_size;drop_nlink (dir);mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -816,7 +818,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -836,6 +838,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -896,7 +899,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change; --2.3.3
Ben Shelton
Reviewed-byQuoted 9 lines · Brian Norris
Quoted 3 lines · Taesoo Kim
When
ubifs_init_security()fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. There are four such places that were introduce by the last commit."The last commit" is not very descriptive. Are you speaking of this commit?
commit
d7f0b70d30ffb9bbe6b8a3e1035cf0b79965ef53
Quoted 8 lines
Author: Subodh Nijsure <[email protected]> Date: Fri Oct 31 13:50:30 2014 -0500UBIFS: Add security.* XATTR support for the UBIFS
CC'ing authors/reviewers.
Brian
This is indeed a bug, and this patch looks like the right fix. Good catch!
Reviewed-by Ben Shelton <[email protected]>
Ben
Quoted 73 lines
Quoted 73 lines
Signed-off-by Taesoo Kim <[email protected]>--- fs/ubifs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)fs/ubifs/dir.c +7
-5@@ -272,7 +272,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -292,6 +292,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -732,7 +733,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex);insert_inode_hash (inode);@@ -757,6 +758,7 @@ out_cancel: dir_ui->ui_size = dir->i_size;drop_nlink (dir);mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -816,7 +818,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;@@ -836,6 +838,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:@@ -896,7 +899,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change; --2.3.3
Artem Bityutskiy
AppliedQuoted 5 lines · Taesoo Kim
When
ubifs_init_security()fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. There are four such places that were introduce by the last commit.Signed-off-by Taesoo Kim <[email protected]>
Pushed to linux-ubifs.git/master, thanks!
PATCH v2 1/1 UBIFS: fix incorrect unlocking handling
In reply to Taesoo KimFrom: Taesoo Kim <[email protected]>
When ubifs_init_security() fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. Fix this.
Signed-off-by Taesoo Kim <[email protected]>
Fixes d7f0b70d30ff ("UBIFS: Add security.* XATTR support for the UBIFS")
Reviewed-by Ben Shelton <[email protected]>
Signed-off-by Uwe Kleine-König <[email protected]>
--- fs/ubifs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
fs/ubifs/dir.c +7 -4
@@ -272,7 +272,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
@@ -292,6 +292,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -732,7 +733,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex);insert_inode_hash (inode);
@@ -757,6 +758,7 @@ out_cancel: dir_ui->ui_size = dir->i_size;drop_nlink (dir);mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -816,7 +818,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
@@ -836,6 +838,7 @@ out_cancel: dir->i_size -= sz_change; dir_ui->ui_size = dir->i_size;mutex_unlock (&dir_ui->ui_mutex); +out_inode:make_bad_inode (inode);iput (inode); out_budg:
@@ -896,7 +899,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, err =ubifs_init_security (dir, inode, &dentry->d_name);if (err) - goto out_cancel; + goto out_inode;mutex_lock (&dir_ui->ui_mutex); dir->i_size += sz_change;
Signature · 1 line
2.1.4
Quoted 9 lines · Uwe Kleine-König
From: Taesoo Kim <[email protected]>
When
ubifs_init_security()fails, 'ui_mutex' is incorrectly unlocked and incorrectly restores 'i_size'. Fix this.Signed-off-by Taesoo Kim <[email protected]>Fixes d7f0b70d30ff ("UBIFS: Add security.* XATTR support for the UBIFS")Reviewed-by Ben Shelton <[email protected]>Signed-off-by Uwe Kleine-König <[email protected]>
OK, I picked your version of the patch, thanks!