<div dir="auto"><div><br><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Wed, Feb 19, 2025, 1:38 PM Fedor Pchelkin <<a href="mailto:pchelkin@ispras.ru">pchelkin@ispras.ru</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
On Wed, 19. Feb 10:25, Rand Deeb wrote:<br>
> The expression "inactags << bmp->db_agl2size" in the function<br>
> dbFinalizeBmap() is computed using int operands. Although the<br>
> values (inactags and db_agl2size) are derived from filesystem<br>
> parameters and are usually small, there is a theoretical risk that<br>
> the shift could overflow a 32-bit int if extreme values occur.<br>
> <br>
> According to the C standard, shifting a signed 32-bit int can lead<br>
> to undefined behavior if the result exceeds its range. In our<br>
> case, an overflow could miscalculate free blocks, potentially<br>
> leading to erroneous filesystem accounting.<br>
> <br>
> To ensure the arithmetic is performed in 64-bit space, we cast<br>
> "inactags" to s64 before shifting. This defensive fix prevents any<br>
> risk of overflow and complies with kernel coding best practices.<br>
> <br>
> Found by Linux Verification Center (<a href="http://linuxtesting.org" rel="noreferrer noreferrer" target="_blank">linuxtesting.org</a>) with SVACE.<br>
> <br>
> Signed-off-by: Rand Deeb <<a href="mailto:rand.sec96@gmail.com" target="_blank" rel="noreferrer">rand.sec96@gmail.com</a>><br>
> ---<br>
<br>
Why is the patch targeted only to 5.10.y? It should go to the mainline<br>
first, no?<br>
<br>
Please check <a href="https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html" rel="noreferrer noreferrer" target="_blank">https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html</a><br>
<br>
>  fs/jfs/jfs_dmap.c | 4 ++--<br>
>  1 file changed, 2 insertions(+), 2 deletions(-)<br>
> <br>
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c<br>
> index eedea23d70ff..3cc10f9bf9f8 100644<br>
> --- a/fs/jfs/jfs_dmap.c<br>
> +++ b/fs/jfs/jfs_dmap.c<br>
> @@ -3728,8 +3728,8 @@ void dbFinalizeBmap(struct inode *ipbmap)<br>
>        * system size is not a multiple of the group size).<br>
>        */<br>
>       inactfree = (inactags && ag_rem) ?<br>
> -         ((inactags - 1) << bmp->db_agl2size) + ag_rem<br>
> -         : inactags << bmp->db_agl2size;<br>
> +         (((s64)inactags - 1) << bmp->db_agl2size) + ag_rem<br>
> +         : ((s64)inactags << bmp->db_agl2size);<br>
>  <br>
>       /* determine how many free blocks are in the active<br>
>        * allocation groups plus the average number of free blocks<br>
> -- <br>
> 2.34.1<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div dir="auto">Hey Fedor,  </div><div dir="auto"><br></div><div dir="auto">I focused on 5.10 and added it to the subject to avoid confusion,  </div><div dir="auto">since files differ across versions. But yes, all versions have the issue.  </div><div dir="auto">In one of my past patches, maintainers couldn't apply it due to kernel  </div><div dir="auto">version differences, which led to confusion. So I thought specifying  </div><div dir="auto">the version upfront would help. My bad, I should have noted it after  </div><div dir="auto">the commit message instead.  </div><div dir="auto"><br></div><div dir="auto">I'll take this into account in future patches. Should I send another  </div><div dir="auto">patch specifically for the mainline version now?  </div><div dir="auto"><br></div><div dir="auto">Thanks for the feedback!</div></div><div dir="auto"><div class="gmail_quote gmail_quote_container"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
</blockquote></div></div></div>