libblkid: Avoid strlen if only first char is checked
PATCH 1/1 libblkid: Avoid strlen if only first char is checked
A strlen() call can lead to out of boundary read access if the superblock in question has no nul-bytes after the string. This could be avoided by using strnlen() but the calls in question merely existed to check if the string length is not 0.
By changing the calls as proposed with this diff, these files are in sync with other superblock files, which do exactly the same.
--- libblkid/src/superblocks/befs.c | 2 +- libblkid/src/superblocks/ext.c | 2 +- libblkid/src/superblocks/jfs.c | 2 +- libblkid/src/superblocks/nilfs.c | 2 +- libblkid/src/superblocks/romfs.c | 2 +- libblkid/src/superblocks/xfs.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
libblkid/src/superblocks/befs.c +1 -1
@@ -451,7 +451,7 @@ static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag)/* * all checks pass, set LABEL, VERSION and UUID */ -if (strlen (bs->name)) +if (*bs->name !='\0' )blkid_probe_set_label (pr, (unsigned char *) bs->name,sizeof (bs->name));if (version)
libblkid/src/superblocks/ext.c +1 -1
@@ -170,7 +170,7 @@ static void ext_get_info(blkid_probe pr, int ver, struct ext2_super_block *es)le32_to_cpu (es->s_feature_incompat), le32_to_cpu(es->s_feature_ro_compat))); -if (strlen (es->s_volume_name)) +if (*es->s_volume_name !='\0' )blkid_probe_set_label (pr, (unsigned char *) es->s_volume_name,sizeof (es->s_volume_name));blkid_probe_set_uuid (pr, es->s_uuid);
libblkid/src/superblocks/jfs.c +1 -1
@@ -49,7 +49,7 @@ static int probe_jfs(blkid_probe pr, const struct blkid_idmag *mag) le16_to_cpu(js->js_l2bfactor))return 1 ; -if (strlen ((char *) js->js_label)) +if (*((char *) js->js_label) !='\0' )blkid_probe_set_label (pr, js->js_label,sizeof (js->js_label));blkid_probe_set_uuid (pr, js->js_uuid);return 0 ;
libblkid/src/superblocks/nilfs.c +1 -1
@@ -143,7 +143,7 @@ static int probe_nilfs2(blkid_probe pr,DBG (LOWPROBE ,ul_debug ("nilfs2: primary=%d, backup=%d, swap=%d" , valid[0 ], valid[1 ], swp)); -if (strlen (sb->s_volume_name)) +if (*(sb->s_volume_name) !='\0' )blkid_probe_set_label (pr, (unsigned char *) sb->s_volume_name,sizeof (sb->s_volume_name));
libblkid/src/superblocks/romfs.c +1 -1
@@ -31,7 +31,7 @@ static int probe_romfs(blkid_probe pr, const struct blkid_idmag *mag)if (!ros)return errno ? -errno :1 ; -if (strlen ((char *) ros->ros_volume)) +if (*((char *) ros->ros_volume) !='\0' )blkid_probe_set_label (pr, ros->ros_volume,sizeof (ros->ros_volume));return 0 ;
libblkid/src/superblocks/xfs.c +1 -1
@@ -169,7 +169,7 @@ static int probe_xfs(blkid_probe pr, const struct blkid_idmag *mag)if (!xfs_verify_sb (xs))return 1 ; -if (strlen (xs->sb_fname)) +if (*xs->sb_fname !='\0' )blkid_probe_set_label (pr, (unsigned char *) xs->sb_fname,sizeof (xs->sb_fname));blkid_probe_set_uuid (pr, xs->sb_uuid);
Signature · 1 line
2.10.0
Quoted 7 lines · Tobias Stoeckmann
A
strlen()call can lead to out of boundary read access if the superblock in question has no nul-bytes after the string. This could be avoided by usingstrnlen()but the calls in question merely existed to check if the string length is not 0.By changing the calls as proposed with this diff, these files are in sync with other superblock files, which do exactly the same.
Quoted 8 lines
--- libblkid/src/superblocks/befs.c | 2 +- libblkid/src/superblocks/ext.c | 2 +- libblkid/src/superblocks/jfs.c | 2 +- libblkid/src/superblocks/nilfs.c | 2 +- libblkid/src/superblocks/romfs.c | 2 +- libblkid/src/superblocks/xfs.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
Applied, thanks.
libblkid/src/superblocks/befs.c static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag) · line 454
Quoted 10 lines
libblkid/src/superblocks/befs.c +1
-1@@ -451,7 +451,7 @@ static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag)/* * all checks pass, set LABEL, VERSION and UUID */ -if (strlen (bs->name)) +if (*bs->name !='\0' )
Good catch, I hate it too. BTW, you can use
if (*bs->name)
it's enough.
Karel
Signature · 2 lines
Karel Zak <[email protected]> http://karelzak.blogspot.com
Quoted 17 lines · Karel Zak
Quoted 7 lines · Tobias Stoeckmann
A
strlen()call can lead to out of boundary read access if the superblock in question has no nul-bytes after the string. This could be avoided by usingstrnlen()but the calls in question merely existed to check if the string length is not 0. =20
By changing the calls as proposed with this diff, these files are in sync with other superblock files, which do exactly the same.Quoted 8 lines
--- libblkid/src/superblocks/befs.c | 2 +- libblkid/src/superblocks/ext.c | 2 +- libblkid/src/superblocks/jfs.c | 2 +- libblkid/src/superblocks/nilfs.c | 2 +- libblkid/src/superblocks/romfs.c | 2 +- libblkid/src/superblocks/xfs.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
Quoted 3 lines
Applied, thanks.
libblkid/src/superblocks/=
Quoted 1 line
libblkid/src/superblocks/= renamed from libblkid/src/superblocks/befs.c +0
-0
befs.c
Quoted 4 lines
libblkid/src/superblocks/befs.c static int probe_befs(blkid_probe pr, const struct b=
Quoted 4 lines
index 7e9eaf6..36e079f 100644
libblkid/src/superblocks/befs.c +0
-0@@ -451,7 +451,7 @@ static int probe_befs(blkid_probe pr, const struct b=
lkid_idmag *mag)
Quoted 11 lines
libblkid/src/superblocks/befs.c static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag)
Quoted 5 lines
libblkid/src/superblocks/befs.c +1
-1/* * all checks pass, set LABEL, VERSION and UUID */ -if (strlen (bs->name)) + if (*bs->name !=3D '\0')Good catch, I hate it too. BTW, you can use
if (*bs->name)it's enough.
Interesting to note that GCC compiles it both to the same instructions:
#include <string.h>
int code1(const char *s) {
if(strlen(s)) return 1;
return 0;
}
int code2(const char *s) {
if(*s) return 1;
return 0;
}
code1(char const*):
push rbp
mov rbp, rsp
mov QWORD PTR [rbp-8], rdi
mov rax, QWORD PTR [rbp-8]
movzx eax, BYTE PTR [rax]
test al, al
je .L2
mov eax, 1
jmp .L3.L2:
mov eax, 0
.L3:
pop rbp
retcode2(char const*):
push rbp
mov rbp, rsp
mov QWORD PTR [rbp-8], rdi
mov rax, QWORD PTR [rbp-8]
movzx eax, BYTE PTR [rax]
test al, al
je .L5
mov eax, 1
jmp .L6.L5:
mov eax, 0
.L6:
pop rbp
ret
--=20
Aur=C3=A9lien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstra=C3=9Fe 5, 90409 N=C3=BCrnberg, Germany
GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton, HRB 21284 (AG N= =C3=BCrnberg)
Quoted 32 lines · Karel Zak
Quoted 7 lines · Tobias Stoeckmann
A
strlen()call can lead to out of boundary read access if the superblock in question has no nul-bytes after the string. This could be avoided by usingstrnlen()but the calls in question merely existed to check if the string length is not 0.By changing the calls as proposed with this diff, these files are in sync with other superblock files, which do exactly the same.
Quoted 8 lines
--- libblkid/src/superblocks/befs.c | 2 +- libblkid/src/superblocks/ext.c | 2 +- libblkid/src/superblocks/jfs.c | 2 +- libblkid/src/superblocks/nilfs.c | 2 +- libblkid/src/superblocks/romfs.c | 2 +- libblkid/src/superblocks/xfs.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)Applied, thanks.
libblkid/src/superblocks/befs.c static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag) · line 454
Quoted 10 lines
libblkid/src/superblocks/befs.c +1
-1@@ -451,7 +451,7 @@ static int probe_befs(blkid_probe pr, const struct blkid_idmag *mag)/* * all checks pass, set LABEL, VERSION and UUID */ -if (strlen (bs->name)) +if (*bs->name !='\0' )Good catch, I hate it too. BTW, you can use
Quoted 3 lines
if (*bs->name)it's enough.
It is enough for the compiler, but the explicit comparison is more clear to a casual reader. The compiler probably optimizes out the comparison anyway.
-- Bruce