| Message ID | 20250413072745.3949-1-huhai@kylinos.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | pstore/zone: Fix return value in psz_init_zone() and psz_init_zones() | expand |
On Sun, Apr 13, 2025 at 03:27:45PM +0800, hhtracer@gmail.com wrote: > From: huhai <huhai@kylinos.cn> > > The psz_init_zone() and psz_init_zones() functions return NULL on error, > but psz_alloc_zones() only checks the return value using IS_ERR(). Since > NULL is not an error pointer, this causes psz_alloc_zones() to mistakenly > treat a failure as success and return 0, which may lead to a NULL pointer > dereference. Have you encountered this failure? > > Update both psz_init_zone() and psz_init_zones() to return proper error > pointers using ERR_PTR() instead of NULL. > > Fixes: d26c3321fe18 ("pstore/zone: Introduce common layer to manage storage zones") > Signed-off-by: huhai <huhai@kylinos.cn> > --- > fs/pstore/zone.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c > index ceb5639a0629..57ffcf76f254 100644 > --- a/fs/pstore/zone.c > +++ b/fs/pstore/zone.c > @@ -1157,7 +1157,7 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type, > const char *name = pstore_type_to_name(type); > > if (!size) > - return NULL; Because as far as I know, this is by design: a size 0 prz means there's no zone. Nothing should ever try to use it because the size is 0. > + return ERR_PTR(-EINVAL); > > if (*off + size > info->total_size) { > pr_err("no room for %s (0x%zx@0x%llx over 0x%lx)\n", > @@ -1203,7 +1203,7 @@ static struct pstore_zone **psz_init_zones(enum pstore_type_id type, > > *cnt = 0; > if (!total_size || !record_size) > - return NULL; > + return ERR_PTR(-EINVAL); > > if (*off + total_size > info->total_size) { > pr_err("no room for zones %s (0x%zx@0x%llx over 0x%lx)\n", > @@ -1225,7 +1225,7 @@ static struct pstore_zone **psz_init_zones(enum pstore_type_id type, > > for (i = 0; i < c; i++) { > zone = psz_init_zone(type, off, record_size); > - if (!zone || IS_ERR(zone)) { > + if (IS_ERR(zone)) { > pr_err("initialize zones %s failed\n", name); > psz_free_zones(&zones, &i); > return (void *)zone; I think the code is correct as-is. Have you found a place where the prz is used even when NULL?
diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c index ceb5639a0629..57ffcf76f254 100644 --- a/fs/pstore/zone.c +++ b/fs/pstore/zone.c @@ -1157,7 +1157,7 @@ static struct pstore_zone *psz_init_zone(enum pstore_type_id type, const char *name = pstore_type_to_name(type); if (!size) - return NULL; + return ERR_PTR(-EINVAL); if (*off + size > info->total_size) { pr_err("no room for %s (0x%zx@0x%llx over 0x%lx)\n", @@ -1203,7 +1203,7 @@ static struct pstore_zone **psz_init_zones(enum pstore_type_id type, *cnt = 0; if (!total_size || !record_size) - return NULL; + return ERR_PTR(-EINVAL); if (*off + total_size > info->total_size) { pr_err("no room for zones %s (0x%zx@0x%llx over 0x%lx)\n", @@ -1225,7 +1225,7 @@ static struct pstore_zone **psz_init_zones(enum pstore_type_id type, for (i = 0; i < c; i++) { zone = psz_init_zone(type, off, record_size); - if (!zone || IS_ERR(zone)) { + if (IS_ERR(zone)) { pr_err("initialize zones %s failed\n", name); psz_free_zones(&zones, &i); return (void *)zone;