* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
2015-03-18 2:09 [PATCH 1/1] UBIFS: fix incorrect unlocking handling Taesoo Kim
@ 2015-03-18 3:18 ` Brian Norris
2015-03-18 3:37 ` Taesoo Kim
2015-03-18 14:54 ` Ben Shelton
2015-03-25 8:41 ` Artem Bityutskiy
2015-03-25 8:53 ` [PATCH v2] " Uwe Kleine-König
2 siblings, 2 replies; 7+ messages in thread
From: Brian Norris @ 2015-03-18 3:18 UTC (permalink / raw)
To: Taesoo Kim
Cc: taesoo, Subodh Nijsure, dedekind1, Ben Shelton, sanidhya,
linux-kernel, adrian.hunter, Terry Wilcox, linux-mtd, blee,
Marc Kleine-Budde, Gratian Crisan, csong84, changwoo,
Brad Mouring
On Tue, Mar 17, 2015 at 10:09:26PM -0400, Taesoo Kim wrote:
> 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 <snijsure@grid-net.com>
Date: Fri Oct 31 13:50:30 2014 -0500
UBIFS: Add security.* XATTR support for the UBIFS
CC'ing authors/reviewers.
Brian
> Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> ---
> fs/ubifs/dir.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 0fa6c80..5b24bc4 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -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
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
2015-03-18 3:18 ` Brian Norris
@ 2015-03-18 3:37 ` Taesoo Kim
2015-03-18 14:54 ` Ben Shelton
1 sibling, 0 replies; 7+ messages in thread
From: Taesoo Kim @ 2015-03-18 3:37 UTC (permalink / raw)
To: Brian Norris
Cc: Subodh Nijsure, dedekind1, Ben Shelton, sanidhya, adrian.hunter,
linux-kernel, Terry Wilcox, linux-mtd, blee, Taesoo Kim,
Marc Kleine-Budde, Gratian Crisan, csong84, changwoo,
Brad Mouring
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
On 03/17/15 at 08:18pm, Brian Norris wrote:
> On Tue, Mar 17, 2015 at 10:09:26PM -0400, Taesoo Kim wrote:
> > 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 <snijsure@grid-net.com>
> Date: Fri Oct 31 13:50:30 2014 -0500
>
> UBIFS: Add security.* XATTR support for the UBIFS
>
> CC'ing authors/reviewers.
>
> Brian
>
> > Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> > ---
> > fs/ubifs/dir.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> > index 0fa6c80..5b24bc4 100644
> > --- a/fs/ubifs/dir.c
> > +++ b/fs/ubifs/dir.c
> > @@ -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
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
2015-03-18 3:18 ` Brian Norris
2015-03-18 3:37 ` Taesoo Kim
@ 2015-03-18 14:54 ` Ben Shelton
1 sibling, 0 replies; 7+ messages in thread
From: Ben Shelton @ 2015-03-18 14:54 UTC (permalink / raw)
To: Brian Norris
Cc: taesoo, Subodh Nijsure, dedekind1, sanidhya, adrian.hunter,
linux-kernel, Terry Wilcox, linux-mtd, blee, Taesoo Kim,
Marc Kleine-Budde, Gratian Crisan, csong84, changwoo,
Brad Mouring
On 03/17, Brian Norris wrote:
> On Tue, Mar 17, 2015 at 10:09:26PM -0400, Taesoo Kim wrote:
> > 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 <snijsure@grid-net.com>
> Date: Fri Oct 31 13:50:30 2014 -0500
>
> UBIFS: 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 <ben.shelton@ni.com>
Ben
>
> > Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> > ---
> > fs/ubifs/dir.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> > index 0fa6c80..5b24bc4 100644
> > --- a/fs/ubifs/dir.c
> > +++ b/fs/ubifs/dir.c
> > @@ -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
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
2015-03-18 2:09 [PATCH 1/1] UBIFS: fix incorrect unlocking handling Taesoo Kim
2015-03-18 3:18 ` Brian Norris
@ 2015-03-25 8:41 ` Artem Bityutskiy
2015-03-25 8:53 ` [PATCH v2] " Uwe Kleine-König
2 siblings, 0 replies; 7+ messages in thread
From: Artem Bityutskiy @ 2015-03-25 8:41 UTC (permalink / raw)
To: Taesoo Kim
Cc: taesoo, sanidhya, adrian.hunter, linux-kernel, linux-mtd, blee,
csong84, changwoo
On Tue, 2015-03-17 at 22:09 -0400, Taesoo Kim wrote:
> 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 <tsgatesv@gmail.com>
Pushed to linux-ubifs.git/master, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] UBIFS: fix incorrect unlocking handling
2015-03-18 2:09 [PATCH 1/1] UBIFS: fix incorrect unlocking handling Taesoo Kim
2015-03-18 3:18 ` Brian Norris
2015-03-25 8:41 ` Artem Bityutskiy
@ 2015-03-25 8:53 ` Uwe Kleine-König
2015-03-25 9:15 ` Artem Bityutskiy
2 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2015-03-25 8:53 UTC (permalink / raw)
To: Taesoo Kim, dedekind1, adrian.hunter, linux-mtd
Cc: taesoo, Subodh Nijsure, Shelton, sanidhya, Terry Wilcox, kernel,
blee, Gratian Crisan, csong84, changwoo, Brad Mouring
From: Taesoo Kim <tsgatesv@gmail.com>
When ubifs_init_security() fails, 'ui_mutex' is incorrectly
unlocked and incorrectly restores 'i_size'. Fix this.
Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
Fixes: d7f0b70d30ff ("UBIFS: Add security.* XATTR support for the UBIFS")
Reviewed-by: Ben Shelton <ben.shelton@ni.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
fs/ubifs/dir.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 0fa6c803992e..5b24bc42cad9 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -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.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] UBIFS: fix incorrect unlocking handling
2015-03-25 8:53 ` [PATCH v2] " Uwe Kleine-König
@ 2015-03-25 9:15 ` Artem Bityutskiy
0 siblings, 0 replies; 7+ messages in thread
From: Artem Bityutskiy @ 2015-03-25 9:15 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: taesoo, Subodh Nijsure, Shelton, sanidhya, adrian.hunter,
Terry Wilcox, linux-mtd, kernel, blee, Taesoo Kim, Gratian Crisan,
csong84, changwoo, Brad Mouring
On Wed, 2015-03-25 at 09:53 +0100, Uwe Kleine-König wrote:
> From: Taesoo Kim <tsgatesv@gmail.com>
>
> When ubifs_init_security() fails, 'ui_mutex' is incorrectly
> unlocked and incorrectly restores 'i_size'. Fix this.
>
> Signed-off-by: Taesoo Kim <tsgatesv@gmail.com>
> Fixes: d7f0b70d30ff ("UBIFS: Add security.* XATTR support for the UBIFS")
> Reviewed-by: Ben Shelton <ben.shelton@ni.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
OK, I picked your version of the patch, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread