* [PATCH] libblkid: Avoid strlen if only first char is checked
@ 2016-10-03 20:05 Tobias Stoeckmann
2016-10-06 12:59 ` Karel Zak
0 siblings, 1 reply; 4+ messages in thread
From: Tobias Stoeckmann @ 2016-10-03 20:05 UTC (permalink / raw)
To: util-linux
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(-)
diff --git a/libblkid/src/superblocks/befs.c b/libblkid/src/superblocks/befs.c
index 7e9eaf6..36e079f 100644
--- a/libblkid/src/superblocks/befs.c
+++ b/libblkid/src/superblocks/befs.c
@@ -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)
diff --git a/libblkid/src/superblocks/ext.c b/libblkid/src/superblocks/ext.c
index 5b1d179..caf82c1 100644
--- a/libblkid/src/superblocks/ext.c
+++ b/libblkid/src/superblocks/ext.c
@@ -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);
diff --git a/libblkid/src/superblocks/jfs.c b/libblkid/src/superblocks/jfs.c
index ac684d8..0f956ef 100644
--- a/libblkid/src/superblocks/jfs.c
+++ b/libblkid/src/superblocks/jfs.c
@@ -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;
diff --git a/libblkid/src/superblocks/nilfs.c b/libblkid/src/superblocks/nilfs.c
index ab0f74c..ee5c5f9 100644
--- a/libblkid/src/superblocks/nilfs.c
+++ b/libblkid/src/superblocks/nilfs.c
@@ -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));
diff --git a/libblkid/src/superblocks/romfs.c b/libblkid/src/superblocks/romfs.c
index 8e63c10..f3e9f8b 100644
--- a/libblkid/src/superblocks/romfs.c
+++ b/libblkid/src/superblocks/romfs.c
@@ -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;
diff --git a/libblkid/src/superblocks/xfs.c b/libblkid/src/superblocks/xfs.c
index 01e9cda..99848f9 100644
--- a/libblkid/src/superblocks/xfs.c
+++ b/libblkid/src/superblocks/xfs.c
@@ -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);
--
2.10.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libblkid: Avoid strlen if only first char is checked
2016-10-03 20:05 [PATCH] libblkid: Avoid strlen if only first char is checked Tobias Stoeckmann
@ 2016-10-06 12:59 ` Karel Zak
2016-10-06 13:22 ` Aurélien Aptel
2016-10-06 19:02 ` Bruce Dubbs
0 siblings, 2 replies; 4+ messages in thread
From: Karel Zak @ 2016-10-06 12:59 UTC (permalink / raw)
To: Tobias Stoeckmann; +Cc: util-linux
On Mon, Oct 03, 2016 at 10:05:03PM +0200, Tobias Stoeckmann wrote:
> 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(-)
Applied, thanks.
> diff --git a/libblkid/src/superblocks/befs.c b/libblkid/src/superblocks/befs.c
> index 7e9eaf6..36e079f 100644
> --- a/libblkid/src/superblocks/befs.c
> +++ b/libblkid/src/superblocks/befs.c
> @@ -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
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libblkid: Avoid strlen if only first char is checked
2016-10-06 12:59 ` Karel Zak
@ 2016-10-06 13:22 ` Aurélien Aptel
2016-10-06 19:02 ` Bruce Dubbs
1 sibling, 0 replies; 4+ messages in thread
From: Aurélien Aptel @ 2016-10-06 13:22 UTC (permalink / raw)
To: Karel Zak, Tobias Stoeckmann; +Cc: util-linux
Karel Zak <kzak@redhat.com> writes:
> On Mon, Oct 03, 2016 at 10:05:03PM +0200, Tobias Stoeckmann wrote:
>> 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.
>>=20
>> 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(-)
>
> Applied, thanks.
>
>> diff --git a/libblkid/src/superblocks/befs.c b/libblkid/src/superblocks/=
befs.c
>> index 7e9eaf6..36e079f 100644
>> --- a/libblkid/src/superblocks/befs.c
>> +++ b/libblkid/src/superblocks/befs.c
>> @@ -451,7 +451,7 @@ static int probe_befs(blkid_probe pr, const struct b=
lkid_idmag *mag)
>> /*
>> * 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:
https://godbolt.org/g/adKv1I
#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
ret
code2(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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libblkid: Avoid strlen if only first char is checked
2016-10-06 12:59 ` Karel Zak
2016-10-06 13:22 ` Aurélien Aptel
@ 2016-10-06 19:02 ` Bruce Dubbs
1 sibling, 0 replies; 4+ messages in thread
From: Bruce Dubbs @ 2016-10-06 19:02 UTC (permalink / raw)
To: Karel Zak, Tobias Stoeckmann; +Cc: util-linux
Karel Zak wrote:
> On Mon, Oct 03, 2016 at 10:05:03PM +0200, Tobias Stoeckmann wrote:
>> 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(-)
>
> Applied, thanks.
>
>> diff --git a/libblkid/src/superblocks/befs.c b/libblkid/src/superblocks/befs.c
>> index 7e9eaf6..36e079f 100644
>> --- a/libblkid/src/superblocks/befs.c
>> +++ b/libblkid/src/superblocks/befs.c
>> @@ -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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-06 19:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-03 20:05 [PATCH] libblkid: Avoid strlen if only first char is checked Tobias Stoeckmann
2016-10-06 12:59 ` Karel Zak
2016-10-06 13:22 ` Aurélien Aptel
2016-10-06 19:02 ` Bruce Dubbs
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.