All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-18  2:09 ` Taesoo Kim
  0 siblings, 0 replies; 12+ messages in thread
From: Taesoo Kim @ 2015-03-18  2:09 UTC (permalink / raw)
  To: dedekind1, adrian.hunter, linux-mtd, linux-kernel
  Cc: taesoo, changwoo, sanidhya, blee, csong84, 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 <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 related	[flat|nested] 12+ messages in thread

* [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-18  2:09 ` Taesoo Kim
  0 siblings, 0 replies; 12+ messages in thread
From: Taesoo Kim @ 2015-03-18  2:09 UTC (permalink / raw)
  To: dedekind1, adrian.hunter, linux-mtd, linux-kernel
  Cc: taesoo, sanidhya, Taesoo Kim, blee, csong84, changwoo

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>
---
 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 related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
  2015-03-18  2:09 ` Taesoo Kim
@ 2015-03-18  3:18   ` Brian Norris
  -1 siblings, 0 replies; 12+ messages in thread
From: Brian Norris @ 2015-03-18  3:18 UTC (permalink / raw)
  To: Taesoo Kim
  Cc: dedekind1, adrian.hunter, linux-mtd, linux-kernel, taesoo,
	sanidhya, blee, csong84, changwoo, Subodh Nijsure,
	Marc Kleine-Budde, Ben Shelton, Brad Mouring, Terry Wilcox,
	Gratian Crisan

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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-18  3:18   ` Brian Norris
  0 siblings, 0 replies; 12+ 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] 12+ 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
  -1 siblings, 0 replies; 12+ messages in thread
From: Taesoo Kim @ 2015-03-18  3:37 UTC (permalink / raw)
  To: Brian Norris
  Cc: Taesoo Kim, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	sanidhya, blee, csong84, changwoo, Subodh Nijsure,
	Marc Kleine-Budde, Ben Shelton, Brad Mouring, Terry Wilcox,
	Gratian Crisan

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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-18  3:37     ` Taesoo Kim
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
  2015-03-18  3:18   ` Brian Norris
@ 2015-03-18 14:54     ` Ben Shelton
  -1 siblings, 0 replies; 12+ messages in thread
From: Ben Shelton @ 2015-03-18 14:54 UTC (permalink / raw)
  To: Brian Norris
  Cc: Taesoo Kim, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	taesoo, sanidhya, blee, csong84, changwoo, Subodh Nijsure,
	Marc Kleine-Budde, Brad Mouring, Terry Wilcox, Gratian Crisan

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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-18 14:54     ` Ben Shelton
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
  2015-03-18  2:09 ` Taesoo Kim
@ 2015-03-25  8:41   ` Artem Bityutskiy
  -1 siblings, 0 replies; 12+ messages in thread
From: Artem Bityutskiy @ 2015-03-25  8:41 UTC (permalink / raw)
  To: Taesoo Kim
  Cc: adrian.hunter, linux-mtd, linux-kernel, taesoo, changwoo,
	sanidhya, blee, csong84

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] 12+ messages in thread

* Re: [PATCH 1/1] UBIFS: fix incorrect unlocking handling
@ 2015-03-25  8:41   ` Artem Bityutskiy
  0 siblings, 0 replies; 12+ 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] 12+ messages in thread

* [PATCH v2] UBIFS: fix incorrect unlocking handling
  2015-03-18  2:09 ` Taesoo Kim
                   ` (2 preceding siblings ...)
  (?)
@ 2015-03-25  8:53 ` Uwe Kleine-König
  2015-03-25  9:15   ` Artem Bityutskiy
  -1 siblings, 1 reply; 12+ 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] 12+ 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; 12+ 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] 12+ messages in thread

end of thread, other threads:[~2015-03-25  9:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18  2:09 [PATCH 1/1] UBIFS: fix incorrect unlocking handling Taesoo Kim
2015-03-18  2:09 ` Taesoo Kim
2015-03-18  3:18 ` Brian Norris
2015-03-18  3:18   ` Brian Norris
2015-03-18  3:37   ` Taesoo Kim
2015-03-18  3:37     ` Taesoo Kim
2015-03-18 14:54   ` Ben Shelton
2015-03-18 14:54     ` Ben Shelton
2015-03-25  8:41 ` Artem Bityutskiy
2015-03-25  8:41   ` Artem Bityutskiy
2015-03-25  8:53 ` [PATCH v2] " Uwe Kleine-König
2015-03-25  9:15   ` Artem Bityutskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.