Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
Signed-off-by: Denis Efremov <[email protected]>
---
I'm not sure about TODO comment, through.
fs/udf/super.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 1c42f544096d..c7cd15219b7c 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1013,10 +1013,7 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
size = sizeof(struct udf_bitmap) +
(sizeof(struct buffer_head *) * nr_groups);
- if (size <= PAGE_SIZE)
- bitmap = kzalloc(size, GFP_KERNEL);
- else
- bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
+ bitmap = kvzalloc(size, GFP_KERNEL);
if (!bitmap)
return NULL;
--
2.26.2
> @@ -1013,10 +1013,7 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
> size = sizeof(struct udf_bitmap) +
> (sizeof(struct buffer_head *) * nr_groups);
I missed that this size is a good place to use struct_size for
overflow checking. I will send v2 instead.
On Thu, 2020-08-27 at 18:28 +0300, Denis Efremov wrote:
> > @@ -1013,10 +1013,7 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
> > size = sizeof(struct udf_bitmap) +
> > (sizeof(struct buffer_head *) * nr_groups);
>
> I missed that this size is a good place to use struct_size for
> overflow checking. I will send v2 instead.
And you could cuddle the
if (!bitmap)
return NULL;
by removing the blank line between the alloc and test.
Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
Size computation wrapped in struct_size() macro to prevent potential
integer overflows.
Signed-off-by: Denis Efremov <[email protected]>
---
fs/udf/super.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 1c42f544096d..bdf51bea54f3 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1010,14 +1010,9 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
int size;
nr_groups = udf_compute_nr_groups(sb, index);
- size = sizeof(struct udf_bitmap) +
- (sizeof(struct buffer_head *) * nr_groups);
-
- if (size <= PAGE_SIZE)
- bitmap = kzalloc(size, GFP_KERNEL);
- else
- bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
+ size = struct_size(bitmap, s_block_bitmap, nr_groups);
+ bitmap = kvzalloc(size, GFP_KERNEL);
if (!bitmap)
return NULL;
--
2.26.2
On 8/28/20 1:09 AM, Gustavo A. R. Silva wrote:
>
>
> On 8/27/20 16:25, Denis Efremov wrote:
>> Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
>> Size computation wrapped in struct_size() macro to prevent potential
>> integer overflows.
>>
>> Signed-off-by: Denis Efremov <[email protected]>
>> ---
>
> Please, comment here what changed in v2, vn... e.g.:
>
> Changes in v2:
> - Use struct_size() helper.
>
Ah, thanks. I added this initially and accidentally regenerated the patch
file with format-patch.
>
> Why not this:
>
> bitmap = kvzalloc(struct_size(bitmap, s_block_bitmap, nr_groups),
> GFP_KERNEL);
>
> and you can also get rid of _size_ entirely.
>
My bad, I missed that only nr_groups is used down the code.
Thanks, I will resend it as v3.
Denis
Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
Size computation wrapped in struct_size() macro to prevent potential
integer overflows.
Signed-off-by: Denis Efremov <[email protected]>
---
Changes in v2:
- size computation wrapped in struct_size()
Changes in v3:
- int size dropped
fs/udf/super.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 1c42f544096d..d9eabbe368ff 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1006,18 +1006,10 @@ int udf_compute_nr_groups(struct super_block *sb, u32 partition)
static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
{
struct udf_bitmap *bitmap;
- int nr_groups;
- int size;
-
- nr_groups = udf_compute_nr_groups(sb, index);
- size = sizeof(struct udf_bitmap) +
- (sizeof(struct buffer_head *) * nr_groups);
-
- if (size <= PAGE_SIZE)
- bitmap = kzalloc(size, GFP_KERNEL);
- else
- bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
+ int nr_groups = udf_compute_nr_groups(sb, index);
+ bitmap = kvzalloc(struct_size(bitmap, s_block_bitmap, nr_groups),
+ GFP_KERNEL);
if (!bitmap)
return NULL;
--
2.26.2
On 8/27/20 16:25, Denis Efremov wrote:
> Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
> Size computation wrapped in struct_size() macro to prevent potential
> integer overflows.
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
Please, comment here what changed in v2, vn... e.g.:
Changes in v2:
- Use struct_size() helper.
> fs/udf/super.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 1c42f544096d..bdf51bea54f3 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -1010,14 +1010,9 @@ static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
> int size;
>
> nr_groups = udf_compute_nr_groups(sb, index);
> - size = sizeof(struct udf_bitmap) +
> - (sizeof(struct buffer_head *) * nr_groups);
> -
> - if (size <= PAGE_SIZE)
> - bitmap = kzalloc(size, GFP_KERNEL);
> - else
> - bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
> + size = struct_size(bitmap, s_block_bitmap, nr_groups);
>
> + bitmap = kvzalloc(size, GFP_KERNEL);
Why not this:
bitmap = kvzalloc(struct_size(bitmap, s_block_bitmap, nr_groups),
GFP_KERNEL);
and you can also get rid of _size_ entirely.
Thanks
--
Gustavo
> if (!bitmap)
> return NULL;
>
>
On 8/27/20 17:16, Denis Efremov wrote:
> Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
> Size computation wrapped in struct_size() macro to prevent potential
> integer overflows.
>
> Signed-off-by: Denis Efremov <[email protected]>
Looks good. :)
Reviewed-by: Gustavo A. R. Silva <[email protected]>
Thanks
--
Gustavo
> ---
> Changes in v2:
> - size computation wrapped in struct_size()
> Changes in v3:
> - int size dropped
>
> fs/udf/super.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 1c42f544096d..d9eabbe368ff 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -1006,18 +1006,10 @@ int udf_compute_nr_groups(struct super_block *sb, u32 partition)
> static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
> {
> struct udf_bitmap *bitmap;
> - int nr_groups;
> - int size;
> -
> - nr_groups = udf_compute_nr_groups(sb, index);
> - size = sizeof(struct udf_bitmap) +
> - (sizeof(struct buffer_head *) * nr_groups);
> -
> - if (size <= PAGE_SIZE)
> - bitmap = kzalloc(size, GFP_KERNEL);
> - else
> - bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
> + int nr_groups = udf_compute_nr_groups(sb, index);
>
> + bitmap = kvzalloc(struct_size(bitmap, s_block_bitmap, nr_groups),
> + GFP_KERNEL);
> if (!bitmap)
> return NULL;
>
>
On Thu 27-08-20 17:29:25, Gustavo A. R. Silva wrote:
>
>
> On 8/27/20 17:16, Denis Efremov wrote:
> > Use kvzalloc() in udf_sb_alloc_bitmap() instead of open-coding it.
> > Size computation wrapped in struct_size() macro to prevent potential
> > integer overflows.
> >
> > Signed-off-by: Denis Efremov <[email protected]>
>
> Looks good. :)
>
> Reviewed-by: Gustavo A. R. Silva <[email protected]>
Thank you both. I've added the patch to my tree.
Honza
>
> Thanks
> --
> Gustavo
>
> > ---
> > Changes in v2:
> > - size computation wrapped in struct_size()
> > Changes in v3:
> > - int size dropped
> >
> > fs/udf/super.c | 14 +++-----------
> > 1 file changed, 3 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/udf/super.c b/fs/udf/super.c
> > index 1c42f544096d..d9eabbe368ff 100644
> > --- a/fs/udf/super.c
> > +++ b/fs/udf/super.c
> > @@ -1006,18 +1006,10 @@ int udf_compute_nr_groups(struct super_block *sb, u32 partition)
> > static struct udf_bitmap *udf_sb_alloc_bitmap(struct super_block *sb, u32 index)
> > {
> > struct udf_bitmap *bitmap;
> > - int nr_groups;
> > - int size;
> > -
> > - nr_groups = udf_compute_nr_groups(sb, index);
> > - size = sizeof(struct udf_bitmap) +
> > - (sizeof(struct buffer_head *) * nr_groups);
> > -
> > - if (size <= PAGE_SIZE)
> > - bitmap = kzalloc(size, GFP_KERNEL);
> > - else
> > - bitmap = vzalloc(size); /* TODO: get rid of vzalloc */
> > + int nr_groups = udf_compute_nr_groups(sb, index);
> >
> > + bitmap = kvzalloc(struct_size(bitmap, s_block_bitmap, nr_groups),
> > + GFP_KERNEL);
> > if (!bitmap)
> > return NULL;
> >
> >
>
--
Jan Kara <[email protected]>
SUSE Labs, CR