| Message ID | 20240704102617.1132337-4-hadess@hadess.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix a number of static analysis issues #5 | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 4: B1 Line exceeds max length (109>80): "bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv"." 5: B1 Line exceeds max length (127>80): "bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to." 7: B3 Line contains hard tab characters (\t): "1113| if (wordexp(rl_line_buffer, &w, WRDE_NOCMD))" 8: B3 Line contains hard tab characters (\t): "1114|-> return NULL;" 10: B3 Line contains hard tab characters (\t): "1116| matches = menu_completion(default_menu, text, w.we_wordc," 13: B1 Line exceeds max length (109>80): "bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv"." 14: B1 Line exceeds max length (127>80): "bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to." 15: B3 Line contains hard tab characters (\t): "1413| switch (err) {" 16: B3 Line contains hard tab characters (\t): "1414| case WRDE_BADCHAR:" 17: B3 Line contains hard tab characters (\t): "1415|-> return -EBADMSG;" 18: B3 Line contains hard tab characters (\t): "1416| case WRDE_BADVAL:" 19: B3 Line contains hard tab characters (\t): "1417| case WRDE_SYNTAX:" 22: B1 Line exceeds max length (109>80): "bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv"." 23: B1 Line exceeds max length (127>80): "bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to." 24: B3 Line contains hard tab characters (\t): "1416| case WRDE_BADVAL:" 25: B3 Line contains hard tab characters (\t): "1417| case WRDE_SYNTAX:" 26: B3 Line contains hard tab characters (\t): "1418|-> return -EINVAL;" 27: B3 Line contains hard tab characters (\t): "1419| case WRDE_NOSPACE:" 28: B3 Line contains hard tab characters (\t): "1420| return -ENOMEM;" 31: B1 Line exceeds max length (109>80): "bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv"." 32: B1 Line exceeds max length (127>80): "bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to." 33: B3 Line contains hard tab characters (\t): "1418| return -EINVAL;" 34: B3 Line contains hard tab characters (\t): "1419| case WRDE_NOSPACE:" 35: B3 Line contains hard tab characters (\t): "1420|-> return -ENOMEM;" 36: B3 Line contains hard tab characters (\t): "1421| case WRDE_CMDSUB:" 37: B3 Line contains hard tab characters (\t): "1422| if (wordexp(input, &w, 0))" 40: B1 Line exceeds max length (109>80): "bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv"." 41: B1 Line exceeds max length (127>80): "bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to." 42: B3 Line contains hard tab characters (\t): "1421| case WRDE_CMDSUB:" 43: B3 Line contains hard tab characters (\t): "1422| if (wordexp(input, &w, 0))" 44: B3 Line contains hard tab characters (\t): "1423|-> return -ENOEXEC;" 45: B3 Line contains hard tab characters (\t): "1424| break;" 46: B3 Line contains hard tab characters (\t): "1425| };" |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
Hi Bastien, On Thu, Jul 4, 2024 at 6:33 AM Bastien Nocera <hadess@hadess.net> wrote: > > Error: RESOURCE_LEAK (CWE-772): [#def40] [important] > bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". > bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. > 1112| > 1113| if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) > 1114|-> return NULL; Derr, this is NOCMD has been found... > 1115| > 1116| matches = menu_completion(default_menu, text, w.we_wordc, > > Error: RESOURCE_LEAK (CWE-772): [#def42] [important] > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". > bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. > 1413| switch (err) { > 1414| case WRDE_BADCHAR: > 1415|-> return -EBADMSG; > 1416| case WRDE_BADVAL: > 1417| case WRDE_SYNTAX: Ok, but where in the documentation of wordexp it is saying that we_wordv is left with anything allocated if it fails? Ive assumed if it returns an error no argument has been processed, otherwise this is sort of misleading and the errors shall be returned by index of the word. > Error: RESOURCE_LEAK (CWE-772): [#def43] [important] > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". > bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. > 1416| case WRDE_BADVAL: > 1417| case WRDE_SYNTAX: > 1418|-> return -EINVAL; > 1419| case WRDE_NOSPACE: > 1420| return -ENOMEM; > > Error: RESOURCE_LEAK (CWE-772): [#def44] [important] > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". > bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. > 1418| return -EINVAL; > 1419| case WRDE_NOSPACE: > 1420|-> return -ENOMEM; > 1421| case WRDE_CMDSUB: > 1422| if (wordexp(input, &w, 0)) > > Error: RESOURCE_LEAK (CWE-772): [#def45] [important] > bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". > bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. > 1421| case WRDE_CMDSUB: > 1422| if (wordexp(input, &w, 0)) > 1423|-> return -ENOEXEC; > 1424| break; > 1425| }; > --- > src/shared/shell.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/src/shared/shell.c b/src/shared/shell.c > index 878be140c336..c09d41ee54df 100644 > --- a/src/shared/shell.c > +++ b/src/shared/shell.c > @@ -1117,8 +1117,10 @@ static char **shell_completion(const char *text, int start, int end) > if (start > 0) { > wordexp_t w; > > - if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) > + if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) { > + wordfree(&w); > return NULL; > + } > > matches = menu_completion(default_menu, text, w.we_wordc, > w.we_wordv[0]); > @@ -1421,15 +1423,20 @@ int bt_shell_exec(const char *input) > err = wordexp(input, &w, WRDE_NOCMD); > switch (err) { > case WRDE_BADCHAR: > + wordfree(&w); > return -EBADMSG; > case WRDE_BADVAL: > case WRDE_SYNTAX: > + wordfree(&w); > return -EINVAL; > case WRDE_NOSPACE: > + wordfree(&w); > return -ENOMEM; > case WRDE_CMDSUB: > - if (wordexp(input, &w, 0)) > + if (wordexp(input, &w, 0)) { > + wordfree(&w); > return -ENOEXEC; > + } > break; > }; If we really need to call wordfree regardless then I'd probably have a function that wraps wordexp and automatically does wordfree on error.
On Thu, 2024-07-04 at 21:26 -0400, Luiz Augusto von Dentz wrote: > Hi Bastien, > > On Thu, Jul 4, 2024 at 6:33 AM Bastien Nocera <hadess@hadess.net> > wrote: > > > > Error: RESOURCE_LEAK (CWE-772): [#def40] [important] > > bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" > > allocates memory that is stored into "w.we_wordv". > > bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" > > going out of scope leaks the storage "w.we_wordv" points to. > > 1112| > > 1113| if (wordexp(rl_line_buffer, &w, > > WRDE_NOCMD)) > > 1114|-> return NULL; > > Derr, this is NOCMD has been found... Still needs parsing *shrug* > > > 1115| > > 1116| matches = menu_completion(default_menu, > > text, w.we_wordc, > > > > Error: RESOURCE_LEAK (CWE-772): [#def42] [important] > > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" > > allocates memory that is stored into "w.we_wordv". > > bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" > > going out of scope leaks the storage "w.we_wordv" points to. > > 1413| switch (err) { > > 1414| case WRDE_BADCHAR: > > 1415|-> return -EBADMSG; > > 1416| case WRDE_BADVAL: > > 1417| case WRDE_SYNTAX: > > Ok, but where in the documentation of wordexp it is saying that > we_wordv is left with anything allocated if it fails? Ive assumed if > it returns an error no argument has been processed, otherwise this is > sort of misleading and the errors shall be returned by index of the > word. There's nothing says it frees wordv on error, and the code shows it doesn't: https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/wordexp.c;h=a7362ef31b05280001e961c3a630e953110b7397;hb=HEAD#l2203 > > Error: RESOURCE_LEAK (CWE-772): [#def43] [important] > > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" > > allocates memory that is stored into "w.we_wordv". > > bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" > > going out of scope leaks the storage "w.we_wordv" points to. > > 1416| case WRDE_BADVAL: > > 1417| case WRDE_SYNTAX: > > 1418|-> return -EINVAL; > > 1419| case WRDE_NOSPACE: > > 1420| return -ENOMEM; > > > > Error: RESOURCE_LEAK (CWE-772): [#def44] [important] > > bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" > > allocates memory that is stored into "w.we_wordv". > > bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" > > going out of scope leaks the storage "w.we_wordv" points to. > > 1418| return -EINVAL; > > 1419| case WRDE_NOSPACE: > > 1420|-> return -ENOMEM; > > 1421| case WRDE_CMDSUB: > > 1422| if (wordexp(input, &w, 0)) > > > > Error: RESOURCE_LEAK (CWE-772): [#def45] [important] > > bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" > > allocates memory that is stored into "w.we_wordv". > > bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" > > going out of scope leaks the storage "w.we_wordv" points to. > > 1421| case WRDE_CMDSUB: > > 1422| if (wordexp(input, &w, 0)) > > 1423|-> return -ENOEXEC; > > 1424| break; > > 1425| }; > > --- > > src/shared/shell.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/src/shared/shell.c b/src/shared/shell.c > > index 878be140c336..c09d41ee54df 100644 > > --- a/src/shared/shell.c > > +++ b/src/shared/shell.c > > @@ -1117,8 +1117,10 @@ static char **shell_completion(const char > > *text, int start, int end) > > if (start > 0) { > > wordexp_t w; > > > > - if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) > > + if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) { > > + wordfree(&w); > > return NULL; > > + } > > > > matches = menu_completion(default_menu, text, > > w.we_wordc, > > > > w.we_wordv[0]); > > @@ -1421,15 +1423,20 @@ int bt_shell_exec(const char *input) > > err = wordexp(input, &w, WRDE_NOCMD); > > switch (err) { > > case WRDE_BADCHAR: > > + wordfree(&w); > > return -EBADMSG; > > case WRDE_BADVAL: > > case WRDE_SYNTAX: > > + wordfree(&w); > > return -EINVAL; > > case WRDE_NOSPACE: > > + wordfree(&w); > > return -ENOMEM; > > case WRDE_CMDSUB: > > - if (wordexp(input, &w, 0)) > > + if (wordexp(input, &w, 0)) { > > + wordfree(&w); > > return -ENOEXEC; > > + } > > break; > > }; > > If we really need to call wordfree regardless then I'd probably have > a > function that wraps wordexp and automatically does wordfree on error. OK. >
diff --git a/src/shared/shell.c b/src/shared/shell.c index 878be140c336..c09d41ee54df 100644 --- a/src/shared/shell.c +++ b/src/shared/shell.c @@ -1117,8 +1117,10 @@ static char **shell_completion(const char *text, int start, int end) if (start > 0) { wordexp_t w; - if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) + if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) { + wordfree(&w); return NULL; + } matches = menu_completion(default_menu, text, w.we_wordc, w.we_wordv[0]); @@ -1421,15 +1423,20 @@ int bt_shell_exec(const char *input) err = wordexp(input, &w, WRDE_NOCMD); switch (err) { case WRDE_BADCHAR: + wordfree(&w); return -EBADMSG; case WRDE_BADVAL: case WRDE_SYNTAX: + wordfree(&w); return -EINVAL; case WRDE_NOSPACE: + wordfree(&w); return -ENOMEM; case WRDE_CMDSUB: - if (wordexp(input, &w, 0)) + if (wordexp(input, &w, 0)) { + wordfree(&w); return -ENOEXEC; + } break; };
Error: RESOURCE_LEAK (CWE-772): [#def40] [important] bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. 1112| 1113| if (wordexp(rl_line_buffer, &w, WRDE_NOCMD)) 1114|-> return NULL; 1115| 1116| matches = menu_completion(default_menu, text, w.we_wordc, Error: RESOURCE_LEAK (CWE-772): [#def42] [important] bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. 1413| switch (err) { 1414| case WRDE_BADCHAR: 1415|-> return -EBADMSG; 1416| case WRDE_BADVAL: 1417| case WRDE_SYNTAX: Error: RESOURCE_LEAK (CWE-772): [#def43] [important] bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. 1416| case WRDE_BADVAL: 1417| case WRDE_SYNTAX: 1418|-> return -EINVAL; 1419| case WRDE_NOSPACE: 1420| return -ENOMEM; Error: RESOURCE_LEAK (CWE-772): [#def44] [important] bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. 1418| return -EINVAL; 1419| case WRDE_NOSPACE: 1420|-> return -ENOMEM; 1421| case WRDE_CMDSUB: 1422| if (wordexp(input, &w, 0)) Error: RESOURCE_LEAK (CWE-772): [#def45] [important] bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv". bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to. 1421| case WRDE_CMDSUB: 1422| if (wordexp(input, &w, 0)) 1423|-> return -ENOEXEC; 1424| break; 1425| }; --- src/shared/shell.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)