| Message ID | 20240122210546.3423784-2-pctammela@mojatatu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | some musl fixes | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Mon, 22 Jan 2024 18:05:45 -0300 Pedro Tammela <pctammela@mojatatu.com> wrote: > NULL is passed in the format string when nothing is to be printed. > This is commonly done in the print_bool function when a flag is false. > Glibc seems to handle this case nicely but for musl it will cause a > segmentation fault. > > The following is an example of one crash in musl based systems/containers: > tc qdisc add dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 > tc qdisc replace dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 min 100 > tc qdisc show dev dummy0 > > Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> > --- > lib/color.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Wouldn't it be simpler to just add a short circuit check. This would fix the color case as well. diff --git a/lib/color.c b/lib/color.c index 59976847295c..cd0f9f7509b5 100644 --- a/lib/color.c +++ b/lib/color.c @@ -140,6 +140,9 @@ int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...) int ret = 0; va_list args; + if (fmt == NULL) + return 0; + va_start(args, fmt); if (!color_is_enabled || attr == COLOR_NONE) {
On 22/01/2024 21:43, Stephen Hemminger wrote: > On Mon, 22 Jan 2024 18:05:45 -0300 > Pedro Tammela <pctammela@mojatatu.com> wrote: > >> NULL is passed in the format string when nothing is to be printed. >> This is commonly done in the print_bool function when a flag is false. >> Glibc seems to handle this case nicely but for musl it will cause a >> segmentation fault. >> >> The following is an example of one crash in musl based systems/containers: >> tc qdisc add dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 >> tc qdisc replace dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 min 100 >> tc qdisc show dev dummy0 >> >> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> >> --- >> lib/color.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) > > Wouldn't it be simpler to just add a short circuit check. Agreed > This would fix the color case as well. I missed that one, thanks for catching it!
diff --git a/lib/color.c b/lib/color.c index 59976847..027e1703 100644 --- a/lib/color.c +++ b/lib/color.c @@ -143,7 +143,7 @@ int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...) va_start(args, fmt); if (!color_is_enabled || attr == COLOR_NONE) { - ret = vfprintf(fp, fmt, args); + ret = vfprintf(fp, fmt ?: "", args); goto end; }
NULL is passed in the format string when nothing is to be printed. This is commonly done in the print_bool function when a flag is false. Glibc seems to handle this case nicely but for musl it will cause a segmentation fault. The following is an example of one crash in musl based systems/containers: tc qdisc add dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 tc qdisc replace dev dummy0 handle 1: root choke limit 1000 bandwidth 10000 min 100 tc qdisc show dev dummy0 Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> --- lib/color.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)