2019-04-15 15:29:08

by Wenwen Wang

Subject: [PATCH] udf: fix an uninitialized read bug

In udf_lookup(), the pointer 'fi' is a local variable initialized by the
return value of the function call udf_find_entry(). However, if the macro
'UDF_RECOVERY' is defined, this variable will become uninitialized if the
else branch is not taken, which can potentially cause incorrect results in
the following execution.

This patch simply initializes this local pointer to NULL.

Signed-off-by: Wenwen Wang <[email protected]>
---
fs/udf/namei.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 58cc241..9d499e1 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -299,7 +299,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
struct inode *inode = NULL;
struct fileIdentDesc cfi;
struct udf_fileident_bh fibh;
- struct fileIdentDesc *fi;
+ struct fileIdentDesc *fi = NULL;

if (dentry->d_name.len > UDF_NAME_LEN)
return ERR_PTR(-ENAMETOOLONG);
--
2.7.4

2019-04-15 16:06:52

by Jan Kara

Subject: Re: [PATCH] udf: fix an uninitialized read bug

On Mon 15-04-19 10:26:24, Wenwen Wang wrote:
> In udf_lookup(), the pointer 'fi' is a local variable initialized by the
> return value of the function call udf_find_entry(). However, if the macro
> 'UDF_RECOVERY' is defined, this variable will become uninitialized if the
> else branch is not taken, which can potentially cause incorrect results in
> the following execution.
>
> This patch simply initializes this local pointer to NULL.
>
> Signed-off-by: Wenwen Wang <[email protected]>

Thanks for the patch! A better fix is to drop the whole UDF_RECOVERY ifdef
and what's in it. It is just dead code anyway.

Honza

> ---
> fs/udf/namei.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> index 58cc241..9d499e1 100644
> --- a/fs/udf/namei.c
> +++ b/fs/udf/namei.c
> @@ -299,7 +299,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
> struct inode *inode = NULL;
> struct fileIdentDesc cfi;
> struct udf_fileident_bh fibh;
> - struct fileIdentDesc *fi;
> + struct fileIdentDesc *fi = NULL;
>
> if (dentry->d_name.len > UDF_NAME_LEN)
> return ERR_PTR(-ENAMETOOLONG);
> --
> 2.7.4
>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2019-04-15 17:44:53

by Wenwen Wang

Subject: Re: [PATCH] udf: fix an uninitialized read bug

Thanks for your prompt reply, Jan! I will rework the patch.

Best regards,
Wenwen

On Mon, Apr 15, 2019 at 11:05 AM Jan Kara <[email protected]> wrote:
>
> On Mon 15-04-19 10:26:24, Wenwen Wang wrote:
> > In udf_lookup(), the pointer 'fi' is a local variable initialized by the
> > return value of the function call udf_find_entry(). However, if the macro
> > 'UDF_RECOVERY' is defined, this variable will become uninitialized if the
> > else branch is not taken, which can potentially cause incorrect results in
> > the following execution.
> >
> > This patch simply initializes this local pointer to NULL.
> >
> > Signed-off-by: Wenwen Wang <[email protected]>
>
> Thanks for the patch! A better fix is to drop the whole UDF_RECOVERY ifdef
> and what's in it. It is just dead code anyway.
>
> Honza
>
> > ---
> > fs/udf/namei.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/udf/namei.c b/fs/udf/namei.c
> > index 58cc241..9d499e1 100644
> > --- a/fs/udf/namei.c
> > +++ b/fs/udf/namei.c
> > @@ -299,7 +299,7 @@ static struct dentry *udf_lookup(struct inode *dir, struct dentry *dentry,
> > struct inode *inode = NULL;
> > struct fileIdentDesc cfi;
> > struct udf_fileident_bh fibh;
> > - struct fileIdentDesc *fi;
> > + struct fileIdentDesc *fi = NULL;
> >
> > if (dentry->d_name.len > UDF_NAME_LEN)
> > return ERR_PTR(-ENAMETOOLONG);
> > --
> > 2.7.4
> >
> >
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR