[PATCH] ash: use memmove instead of mempcpy in subevalvar
While investigating a sporadic crash issue relating to variable substitution in
Alpine Linux, we managed to get a reliable crash when building BusyBox with
ASan,
due to the source and destination overlapping for mempcpy, which resulted in
sporadic data corruption outside ASan.
Per POSIX, memcpy is not allowed to overlap source and destination, as mempcpy
is a GNU-specific extension to mempcpy, the same semantics can be assumed.
Accordingly, we use memmove instead, which does not have this limitation.
v2: Forgot to emulate mempcpy's dest+size return value, fixed.
v3: Incorporate feedback from David Laight.
Signed-off-by: Ariadne Conill
---
shell/ash.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/shell/ash.c b/shell/ash.c
index adb0f223a..8acd2d32e 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7187,7 +7187,8 @@ subevalvar(char *start, char *str, int strloc,
len = orig_len - pos;
if (!quotes) {
- loc = mempcpy(startp, startp + pos, len);
+ memmove(startp, startp + pos, len);
+ loc = startp + len;
} else {
for (vstr = startp; pos != 0; pos--) {
if ((unsigned char)*vstr == CTLESC)
--
2.35.1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH] ash: use memmove instead of mempcpy in subevalvar
On Sat, Feb 26, 2022 at 06:11:30PM +, Ariadne Conill wrote:
> While investigating a sporadic crash issue relating to variable substitution
> in
> Alpine Linux, we managed to get a reliable crash when building BusyBox with
> ASan,
> due to the source and destination overlapping for mempcpy, which resulted in
> sporadic data corruption outside ASan.
>
> Per POSIX, memcpy is not allowed to overlap source and destination, as mempcpy
> is a GNU-specific extension to mempcpy, the same semantics can be assumed.
> Accordingly, we use memmove instead, which does not have this limitation.
>
> Signed-off-by: Ariadne Conill
> ---
> shell/ash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/shell/ash.c b/shell/ash.c
> index adb0f223a..6f256d4c3 100644
> --- a/shell/ash.c
> +++ b/shell/ash.c
> @@ -7187,7 +7187,7 @@ subevalvar(char *start, char *str, int strloc,
> len = orig_len - pos;
>
> if (!quotes) {
> - loc = mempcpy(startp, startp + pos, len);
> + loc = memmove(startp, startp + pos, len);
> } else {
> for (vstr = startp; pos != 0; pos--) {
> if ((unsigned char)*vstr == CTLESC)
> --
> 2.35.1
>
> ___
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
Looks like the returned pointer isn't the same between the two functions;
shouldn't it be `loc = memmove(...) + len` instead ?
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[PATCH] ash: use memmove instead of mempcpy in subevalvar
While investigating a sporadic crash issue relating to variable substitution in
Alpine Linux, we managed to get a reliable crash when building BusyBox with
ASan,
due to the source and destination overlapping for mempcpy, which resulted in
sporadic data corruption outside ASan.
Per POSIX, memcpy is not allowed to overlap source and destination, as mempcpy
is a GNU-specific extension to mempcpy, the same semantics can be assumed.
Accordingly, we use memmove instead, which does not have this limitation.
Signed-off-by: Ariadne Conill
---
shell/ash.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/shell/ash.c b/shell/ash.c
index adb0f223a..6f256d4c3 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -7187,7 +7187,7 @@ subevalvar(char *start, char *str, int strloc,
len = orig_len - pos;
if (!quotes) {
- loc = mempcpy(startp, startp + pos, len);
+ loc = memmove(startp, startp + pos, len);
} else {
for (vstr = startp; pos != 0; pos--) {
if ((unsigned char)*vstr == CTLESC)
--
2.35.1
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
