| Message ID | 20220523164626.858340-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] modpost: fix undefined behavior of is_arm_mapping_symbol() | expand |
On Mon, May 23, 2022 at 9:48 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > The return value of is_arm_mapping_symbol() is unpredictable when > "$" is passed in. > > strchr(3) says: > The strchr() and strrchr() functions return a pointer to the matched > character or NULL if the character is not found. The terminating null > byte is considered part of the string, so that if c is specified as > '\0', these functions return a pointer to the terminator. > > When str[1] is '\0', strchr("axtd", str[1]) is not NULL, and str[2] is > referenced (i.e. buffer overrun). > > Test code > --------- > > char str1[] = "abc"; > char str2[] = "ab"; > > strcpy(str1, "$"); > strcpy(str2, "$"); > > printf("test1: %d\n", is_arm_mapping_symbol(str1)); > printf("test2: %d\n", is_arm_mapping_symbol(str2)); > > Result > ------ > > test1: 0 > test2: 1 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> I guess this is shorter than a call to strlen then conditional call to strchr. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > scripts/mod/modpost.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 6f5c605ab0fb..845bc438ca49 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1179,7 +1179,8 @@ static int secref_whitelist(const struct sectioncheck *mismatch, > > static inline int is_arm_mapping_symbol(const char *str) > { > - return str[0] == '$' && strchr("axtd", str[1]) > + return str[0] == '$' && > + (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x') > && (str[2] == '\0' || str[2] == '.'); > } > > -- > 2.32.0 >
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index 6f5c605ab0fb..845bc438ca49 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1179,7 +1179,8 @@ static int secref_whitelist(const struct sectioncheck *mismatch, static inline int is_arm_mapping_symbol(const char *str) { - return str[0] == '$' && strchr("axtd", str[1]) + return str[0] == '$' && + (str[1] == 'a' || str[1] == 'd' || str[1] == 't' || str[1] == 'x') && (str[2] == '\0' || str[2] == '.'); }
The return value of is_arm_mapping_symbol() is unpredictable when "$" is passed in. strchr(3) says: The strchr() and strrchr() functions return a pointer to the matched character or NULL if the character is not found. The terminating null byte is considered part of the string, so that if c is specified as '\0', these functions return a pointer to the terminator. When str[1] is '\0', strchr("axtd", str[1]) is not NULL, and str[2] is referenced (i.e. buffer overrun). Test code --------- char str1[] = "abc"; char str2[] = "ab"; strcpy(str1, "$"); strcpy(str2, "$"); printf("test1: %d\n", is_arm_mapping_symbol(str1)); printf("test2: %d\n", is_arm_mapping_symbol(str2)); Result ------ test1: 0 test2: 1 Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/mod/modpost.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)