Regarding CVE-2024-26736: afs: Increase buffer size in afs_update_volume_status()

[Jeffrey E Altman]

https://lore.kernel.org/linux-cve-announce/2024040359-CVE-2024-26736-284d@gregkh/T/#u

On 2024-04-03 CVE-2024-26736 was announced in response to the merging of

    commit 6ea38e2aeb72349cad50e38899b0ba6fbcb2af3d
    Author: Daniil Dulov <d.dulov at aladdin.ru>
    Date:   Mon Feb 19 14:39:03 2024 +0000

         afs: Increase buffer size in afs_update_volume_status()

         The max length of volume->vid value is 20 characters.
         So increase idbuf[] size up to 24 to avoid overflow.

         Found by Linux Verification Center (linuxtesting.org) with SVACE.

         [DH: Actually, it's 20 + NUL, so increase it to 24 and use
    snprintf()]

         Fixes: d2ddc776a458 ("afs: Overhaul volume and server record
    caching and fileserver rotation")
         Signed-off-by: Daniil Dulov <d.dulov at aladdin.ru>
         Signed-off-by: David Howells <dhowells at redhat.com>
         Link:
    https://lore.kernel.org/r/20240211150442.3416-1-d.dulov@aladdin.ru/ # v1
         Link:
    https://lore.kernel.org/r/20240212083347.10742-1-d.dulov@aladdin.ru/
    # v2
         Link:
    https://lore.kernel.org/r/20240219143906.138346-3-dhowells@redhat.com
         Signed-off-by: Christian Brauner <brauner at kernel.org>

After a careful examination of the change and the code history I believe 
the referenced "Fixes" commit is incorrect. It should be

    commit 3b6492df4153b8550d347dfc581856138678a231
    Author: David Howells <dhowells at redhat.com>
    Date:   Sat Oct 20 00:57:57 2018 +0100

         afs: Increase to 64-bit volume ID and 96-bit vnode ID for YFS

         Increase the sizes of the volume ID to 64 bits and the vnode ID
    (inode
         number equivalent) to 96 bits to allow the support of YFS.

         This requires the iget comparator to check the vnode->fid
    rather than i_ino
         and i_generation as i_ino is not sufficiently capacious. It
    also requires
         this data to be placed into the vnode cache key for fscache.

         For the moment, just discard the top 32 bits of the vnode ID
    when returning
         it though stat.

         Signed-off-by: David Howells <dhowells at redhat.com>

which was initially merged as part of v4.20-rc1 and not 4.15 as 
indicated by CVE-2024-26736.

commit 3b6492df4153b8550d347dfc581856138678a231 increased the size of 
typedef afs_volid_t from "unsigned int" to "u64" without increasing the 
size of idbuf[] within afs_update_volume_status().  However, since the 
introduction of 3b6492df4153b8550d347dfc581856138678a231 there has yet 
to be any implementation of either RPC YFSVL_GetEntryByName64 or 
YFSVL_GetEntryByID64 which would permit a volume id larger than 32-bits 
to be stored into the struct afs_volume.vid afs_volid_t typed field.  
fs/afs uses the VL_GetEntryByNameU and VL_GetEntryByIDU RPC variants 
which only support 32-bit volume ids.

Therefore, I do not believe that the code present in 
afs_update_volume_status()

    char idbuf[16];

    idsz = sprintf(idbuf, "%llu", volume->vid);

could in practice result in a buffer overflow as indicated by 
CVE-2024-26736 since the C-string generated by all of the possible 
volume id values would fit within 16 bytes.

Sincerely,

Jeffrey Altman