LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication
@ 2021-08-16 16:30 Colin King
  2021-08-16 18:59 ` Kari Argillander
  2021-08-27 16:11 ` Konstantin Komarov
  0 siblings, 2 replies; 3+ messages in thread
From: Colin King @ 2021-08-16 16:30 UTC (permalink / raw)
  To: Konstantin Komarov, ntfs3; +Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The multiplication of the u32 data_size with a int is being performed
using 32 bit arithmetic however the results is being assigned to the
variable nbits that is a size_t (64 bit) value. Fix a potential
integer overflow by casting the u32 value to a size_t before the
multiply to use a size_t sized bit multiply operation.

Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/ntfs3/index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
index 6aa9540ece47..9386c551e208 100644
--- a/fs/ntfs3/index.c
+++ b/fs/ntfs3/index.c
@@ -2012,7 +2012,7 @@ static int indx_shrink(struct ntfs_index *indx, struct ntfs_inode *ni,
 		unsigned long pos;
 		const unsigned long *bm = resident_data(b);
 
-		nbits = le32_to_cpu(b->res.data_size) * 8;
+		nbits = (size_t)le32_to_cpu(b->res.data_size) * 8;
 
 		if (bit >= nbits)
 			return 0;
-- 
2.32.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication
  2021-08-16 16:30 [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication Colin King
@ 2021-08-16 18:59 ` Kari Argillander
  2021-08-27 16:11 ` Konstantin Komarov
  1 sibling, 0 replies; 3+ messages in thread
From: Kari Argillander @ 2021-08-16 18:59 UTC (permalink / raw)
  To: Colin King; +Cc: Konstantin Komarov, ntfs3, kernel-janitors, linux-kernel

On Mon, Aug 16, 2021 at 05:30:25PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The multiplication of the u32 data_size with a int is being performed
> using 32 bit arithmetic however the results is being assigned to the
> variable nbits that is a size_t (64 bit) value. Fix a potential
> integer overflow by casting the u32 value to a size_t before the
> multiply to use a size_t sized bit multiply operation.
> 
> Addresses-Coverity: ("Unintentional integer overflow")
> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  fs/ntfs3/index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index 6aa9540ece47..9386c551e208 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -2012,7 +2012,7 @@ static int indx_shrink(struct ntfs_index *indx, struct ntfs_inode *ni,
>  		unsigned long pos;
>  		const unsigned long *bm = resident_data(b);
>  
> -		nbits = le32_to_cpu(b->res.data_size) * 8;
> +		nbits = (size_t)le32_to_cpu(b->res.data_size) * 8;

nbits is size_t because we might use b->nres.data_size in this function.
Usually nbits is only u32 in other places and also in those is *8. Those
will also overflow as much as this one. So this does not save our asses
if that kind of situation happend. So basically this does not fix
anything just some static analyzer can find it more easily than other
cases.

I'm still not nacking about it. If some static tool is happy by this
change then that might be good thing so that this will not be addressed
again. I also do not ack this. I did not fully review this so of course
there might be some fundemental problem with these *8 multiplications
but I doubt it.

>  
>  		if (bit >= nbits)
>  			return 0;


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication
  2021-08-16 16:30 [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication Colin King
  2021-08-16 18:59 ` Kari Argillander
@ 2021-08-27 16:11 ` Konstantin Komarov
  1 sibling, 0 replies; 3+ messages in thread
From: Konstantin Komarov @ 2021-08-27 16:11 UTC (permalink / raw)
  To: Colin King, ntfs3; +Cc: kernel-janitors, linux-kernel

> From: Colin King <colin.king@canonical.com>
> Sent: Monday, August 16, 2021 7:30 PM
> To: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>; ntfs3@lists.linux.dev
> Cc: kernel-janitors@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication
> 
> From: Colin Ian King <colin.king@canonical.com>
> 
> The multiplication of the u32 data_size with a int is being performed
> using 32 bit arithmetic however the results is being assigned to the
> variable nbits that is a size_t (64 bit) value. Fix a potential
> integer overflow by casting the u32 value to a size_t before the
> multiply to use a size_t sized bit multiply operation.
> 
> Addresses-Coverity: ("Unintentional integer overflow")
> Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  fs/ntfs3/index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ntfs3/index.c b/fs/ntfs3/index.c
> index 6aa9540ece47..9386c551e208 100644
> --- a/fs/ntfs3/index.c
> +++ b/fs/ntfs3/index.c
> @@ -2012,7 +2012,7 @@ static int indx_shrink(struct ntfs_index *indx, struct ntfs_inode *ni,
>  		unsigned long pos;
>  		const unsigned long *bm = resident_data(b);
> 
> -		nbits = le32_to_cpu(b->res.data_size) * 8;
> +		nbits = (size_t)le32_to_cpu(b->res.data_size) * 8;
> 
>  		if (bit >= nbits)
>  			return 0;
> --
> 2.32.0

Hi again, Colin! Applied, thanks.

Best regards,
Konstantin

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-27 16:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16 16:30 [PATCH][next] fs/ntfs3: Fix integer overflow in multiplication Colin King
2021-08-16 18:59 ` Kari Argillander
2021-08-27 16:11 ` Konstantin Komarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).