[PATCH] udf: propagate error from udf_fiiter_add_entry

Tristan Madani posted 1 patch 1 month, 4 weeks ago
fs/udf/namei.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
[PATCH] udf: propagate error from udf_fiiter_add_entry
Posted by Tristan Madani 1 month, 4 weeks ago
From: Tristan Madani <tristan@talencesecurity.com>

udf_fiiter_add_entry() triggers a WARN when it encounters an error
condition but does not propagate the error to callers. This can be
triggered by a corrupted filesystem image.

Replace the WARN with proper error propagation to allow callers to
handle the failure gracefully.

Reported-by: syzbot+969e250fc7983fc7417c@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
---
 fs/udf/namei.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 9a3b7cef36064..2249b79c0e0b2 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -207,14 +207,9 @@ static int udf_expand_dir_adinicb(struct inode *inode, udf_pblk_t *block)
 		udf_fiiter_write_fi(&iter, impuse);
 	}
 	brelse(dbh);
-	/*
-	 * We don't expect the iteration to fail as the directory has been
-	 * already verified to be correct
-	 */
-	WARN_ON_ONCE(ret);
 	udf_fiiter_release(&iter);
 
-	return 0;
+	return ret;
 }
 
 static int udf_fiiter_add_entry(struct inode *dir, struct dentry *dentry,
-- 
2.47.3
Re: [PATCH] udf: propagate error from udf_fiiter_add_entry
Posted by Jan Kara 1 month, 3 weeks ago
On Sat 18-04-26 13:10:35, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> udf_fiiter_add_entry() triggers a WARN when it encounters an error
> condition but does not propagate the error to callers. This can be
> triggered by a corrupted filesystem image.
> 
> Replace the WARN with proper error propagation to allow callers to
> handle the failure gracefully.
> 
> Reported-by: syzbot+969e250fc7983fc7417c@syzkaller.appspotmail.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>

Thanks for the patch. Did you analyze how it happened that the directory
iteration fixing up tags failed? Because if it fails like this, we have
practically wiped the directory which is not great. So if we are missing
some directory consistency check before we start converting it, it would be
good to add it instead of just failing like this later.

								Honza

> ---
>  fs/udf/namei.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 9a3b7cef36064..2249b79c0e0b2 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -207,14 +207,9 @@ static int udf_expand_dir_adinicb(struct inode *inode, udf_pblk_t *block)
>  		udf_fiiter_write_fi(&iter, impuse);
>  	}
>  	brelse(dbh);
> -	/*
> -	 * We don't expect the iteration to fail as the directory has been
> -	 * already verified to be correct
> -	 */
> -	WARN_ON_ONCE(ret);
>  	udf_fiiter_release(&iter);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int udf_fiiter_add_entry(struct inode *dir, struct dentry *dentry,
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR