Steffen Nurpmeso wrote in
<20220907161808.uye_3%[email protected]>:
|Harald van Dijk wrote in
| <[email protected]>:
||On 07/09/2022 13:43, Steffen Nurpmeso wrote:
||> Harald van Dijk wrote in
||> <[email protected]>:
||> ...
||>|The supplied patch appears to fix both #1 and #2, but it has alignment
||>|issues: when UBSAN is enabled, I see a lot of "runtime error: store to
||>|misaligned address <...> for type 'char *', which requires 4 byte
||>|alignment" warnings when using shell arithmetic.
||>
||> ASAN works, UBSAN i have not tried. How can char* require 4 byte
||> alignment?
||
||This message means the char * object itself needs to be aligned on a 4
||byte boundary, and isn't. It doesn't say anything about what the char *
|
|I cannot reproduce this.
...
|But not in C.
Hmmm, busybox supports that in its config! Trying this out
i see.. that you are right.
shell/shexp-arith.h:468:4: runtime error: store to misaligned address
0x7ffe00e0a9ea for type 'char *', which requires 8 byte alignment
0x7ffe00e0a9ea: note: pointer points here
That is missing alignment after casting operator array.
Interesting -- actually my mailer test _does_ catch it, but since
the program is not aborted the result is still valid, and
therefore the test succeeds. The standard error shows it, after
~three pages scrollback. Wow, what a mess.
My bad.
Well i committed that also to my mailer, since you already have
credit there for the
a_main_startup(): drop inherited effective IDs (Harald van
Dijk)
i had seen last year that at least need not be done.
All this cumulates to the following compared to v4. (The
calculation is still fuzzy.)
Ciao.
commit 8ceee89fda5122c7813b418b9e6a28bd5642d7a6 (HEAD -> refs/heads/sharith)
Author: Steffen Nurpmeso <[email protected]>
AuthorDate: 2022-09-07 18:47:59 +0200
Commit: Steffen Nurpmeso <[email protected]>
CommitDate: 2022-09-07 18:47:59 +0200
shell: $(()): sync+fix bugs (Harald van Dijk)
---
shell/shexp-arith.h | 39 +++++++++++++++++++++------------------
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/shell/shexp-arith.h b/shell/shexp-arith.h
index 5f3655b456..2c1d53d252 100644
--- a/shell/shexp-arith.h
+++ b/shell/shexp-arith.h
@@ -235,7 +235,7 @@ struct a_shexp_arith_stack{
};
struct a_shexp_arith_ctx{
- enum a_shexp_arith_error sac_error;
+ u32 sac_error;
boole sac_have_error_track;
u8 sac__pad[3];
s64 sac_rv;
@@ -382,12 +382,12 @@ a_shexp_arith_eval(
a_shexp__arith_eval(&self, exp_buf, exp_len);
*resp = self.sac_rv;
- a_SHEXP_ARITH_L(("< arith_eval %zu <%.*s> -> <%lld> ERR<%d>\n",
+ a_SHEXP_ARITH_L(("< arith_eval %zu <%.*s> -> <%lld> ERR<%u>\n",
exp_len, S(int,exp_len != UZ_MAX ? exp_len : su_cs_len(exp_buf)),
exp_buf, self.sac_rv, self.sac_error));
NYD_OU;
- return self.sac_error;
+ return S(enum a_shexp_arith_error,self.sac_error);
}
static void
@@ -409,28 +409,32 @@ a_shexp__arith_eval(struct a_shexp_arith_ctx *self,
/* Create a single continuous allocation for anything */
/* C99 */{
union {void *v; char *c;} p;
- uz i, j, a;
+ uz i, j, o, a;
/* Done for empty expression */
if((i = a_shexp__arith_ws_squeeze(self, exp_buf, exp_len, NIL)) == 0)
goto jleave;
- ++i;
/* Overflow check: since arithmetic expressions are rarely long enough
* to come near this limit, xxx laxe & fuzzy, not exact; max U32_MAX! */
- if(su_64( i > U32_MAX || ) i >= UZ_MAX / 2 ||
- i >= ((UZ_MAX - (i a_SHEXP_ARITH_ERROR_TRACK( * 2))) /
- ((su_ALIGNOF(*sasp->sas_nums) + sizeof(*sasp->sas_ops) * 2)
- a_SHEXP_ARITH_ERROR_TRACK(
- + sizeof(*sasp->sas_error_track) * 2 ))
- )){
+ if(su_64( i >= U32_MAX || ) i >= UZ_MAX / 2)
+ goto jenomem;
+ ++i;
+ if(i >= ((UZ_MAX - (i a_SHEXP_ARITH_ERROR_TRACK( * 2 ))) /
+ ((su_ALIGNOF(*sasp->sas_nums) +
+ su_ALIGNOF(sizeof(*sasp->sas_ops) * 2))
+ a_SHEXP_ARITH_ERROR_TRACK(
+ + sizeof(*sasp->sas_error_track) * 2 ))
+ )){
+jenomem:
self->sac_error = a_SHEXP_ARITH_ERR_NOMEM;
goto jleave;
}
++i;
j = su_ALIGNOF(*sasp->sas_nums) * (i >> 1);
- a = j + (sizeof(*sasp->sas_ops) * i) +
+ o = su_ALIGNOF(sizeof(*sasp->sas_ops) * i);
+ a = j + o +
a_SHEXP_ARITH_ERROR_TRACK( (sizeof(*sasp->sas_error_track) * i) + )
1 + (i a_SHEXP_ARITH_ERROR_TRACK( * 2 ));
mem_p = p.v = su_LOFI_ALLOC(a);
@@ -442,7 +446,7 @@ a_shexp__arith_eval(struct a_shexp_arith_ctx *self,
sasp->sas_nums = sasp->sas_nums_top = S(struct a_shexp_arith_val*,p.v);
p.c += j;
sasp->sas_ops = sasp->sas_ops_top = S(u16*,p.v);
- p.c += sizeof(*sasp->sas_ops) * i;
+ p.c += o;
a_SHEXP_ARITH_ERROR_TRACK(
sasp->sas_error_track_top = sasp->sas_error_track = S(char**,p.v);
p.c += sizeof(*sasp->sas_error_track) * i;
@@ -451,7 +455,7 @@ a_shexp__arith_eval(struct a_shexp_arith_ctx *self,
ep = ++p.c; /* ++ to copy varnames in !_ARITH_ERROR cases */
i = a_shexp__arith_ws_squeeze(self, exp_buf, exp_len, ep);
varp = &ep[
-#if 0 a_SHEXP_ARITH_ERROR_TRACK( + 1)
+#if 0 a_SHEXP_ARITH_ERROR_TRACK( + 1 )
i + 1
#else
-1
@@ -703,8 +707,7 @@ junapre:
if(op == a_SHEXP_ARITH_OP_COND){
u16 x;
- x = *sasp->sas_ops_top;
- x &= a_SHEXP_ARITH_OP_FLAG_MASK;
+ x = *sasp->sas_ops_top & a_SHEXP_ARITH_OP_FLAG_MASK;
if(x & a_SHEXP_ARITH_OP_FLAG_WHITEOUT){
x ^= a_SHEXP_ARITH_OP_FLAG_WHITEOUT;
x |= a_SHEXP_ARITH_OP_FLAG_OUTER_WHITEOUT;
@@ -731,7 +734,7 @@ junapre:
sasp->sas_nums_top->sav_var = NIL;
}
- if((sasp->sas_nums_top)->sav_val == 0)
+ if(sasp->sas_nums_top->sav_val == 0)
op |= a_SHEXP_ARITH_OP_FLAG_WHITEOUT;
op |= *sasp->sas_ops_top & a_SHEXP_ARITH_OP_FLAG_MASK;
@@ -922,7 +925,7 @@ a_shexp__arith_ws_squeeze(struct a_shexp_arith_ctx *self,
goto jleave;
if(!(su_cs_is_space(c)
a_SHEXP_ARITH_IFS( || su_cs_find_c(ifs_ws, c) != NIL )
- ))
+ ))
break;
}
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
