The max length of volume->vid value is 20 characters.
So increase idbuf[] size up to 20 to avoid overflow.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
Signed-off-by: Daniil Dulov <[email protected]>
---
fs/afs/volume.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/afs/volume.c b/fs/afs/volume.c
index 020ecd45e476..d4a379bfd54e 100644
--- a/fs/afs/volume.c
+++ b/fs/afs/volume.c
@@ -353,7 +353,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
{
struct afs_server_list *new, *old, *discard;
struct afs_vldb_entry *vldb;
- char idbuf[16];
+ char idbuf[20];
int ret, idsz;
_enter("");
--
2.25.1
On 2/12/2024 3:33 AM, Daniil Dulov wrote:
> The max length of volume->vid value is 20 characters.
> So increase idbuf[] size up to 20 to avoid overflow.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
> Signed-off-by: Daniil Dulov <[email protected]>
> ---
> fs/afs/volume.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/afs/volume.c b/fs/afs/volume.c
> index 020ecd45e476..d4a379bfd54e 100644
> --- a/fs/afs/volume.c
> +++ b/fs/afs/volume.c
> @@ -353,7 +353,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
> {
> struct afs_server_list *new, *old, *discard;
> struct afs_vldb_entry *vldb;
> - char idbuf[16];
> + char idbuf[20];
> int ret, idsz;
>
> _enter("");
Reviewed-by: Jeffrey Altman <[email protected]>
Daniil Dulov <[email protected]> wrote:
> The max length of volume->vid value is 20 characters.
> So increase idbuf[] size up to 20 to avoid overflow.
Is that 20 including the NUL? If not, I'd increase it to 24 (it's likely to
get rounded up to a multiple of 4 or 8 by the compiler).
David
I suggest the attached instead.
David
---
diff --git a/fs/afs/volume.c b/fs/afs/volume.c
index 020ecd45e476..af3a3f57c1b3 100644
--- a/fs/afs/volume.c
+++ b/fs/afs/volume.c
@@ -353,7 +353,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
{
struct afs_server_list *new, *old, *discard;
struct afs_vldb_entry *vldb;
- char idbuf[16];
+ char idbuf[24];
int ret, idsz;
_enter("");
@@ -361,7 +361,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
/* We look up an ID by passing it as a decimal string in the
* operation's name parameter.
*/
- idsz = sprintf(idbuf, "%llu", volume->vid);
+ idsz = snprintf(idbuf, sizeof(idbuf), "%llu", volume->vid);
vldb = afs_vl_lookup_vldb(volume->cell, key, idbuf, idsz);
if (IS_ERR(vldb)) {
Hello!
Thank you for your feedback. I agree with the suggested improvement. I will send v3 a bit later today.
-----Original Message-----
From: David Howells [mailto:[email protected]]
Sent: Monday, February 19, 2024 1:34 PM
To: Daniil Dulov <[email protected]>
Cc: [email protected]; Jeffrey E Altman <[email protected]>; [email protected]; Marc Dionne <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH v2] afs: Increase buffer size in afs_update_volume_status()
I suggest the attached instead.
David
---
diff --git a/fs/afs/volume.c b/fs/afs/volume.c index 020ecd45e476..af3a3f57c1b3 100644
--- a/fs/afs/volume.c
+++ b/fs/afs/volume.c
@@ -353,7 +353,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key) {
struct afs_server_list *new, *old, *discard;
struct afs_vldb_entry *vldb;
- char idbuf[16];
+ char idbuf[24];
int ret, idsz;
_enter("");
@@ -361,7 +361,7 @@ static int afs_update_volume_status(struct afs_volume *volume, struct key *key)
/* We look up an ID by passing it as a decimal string in the
* operation's name parameter.
*/
- idsz = sprintf(idbuf, "%llu", volume->vid);
+ idsz = snprintf(idbuf, sizeof(idbuf), "%llu", volume->vid);
vldb = afs_vl_lookup_vldb(volume->cell, key, idbuf, idsz);
if (IS_ERR(vldb)) {
Daniil Dulov <[email protected]> wrote:
> Thank you for your feedback. I agree with the suggested improvement. I will send v3 a bit later today.
I posted an updated patch:
https://lore.kernel.org/linux-fsdevel/[email protected]/T/#u