Re: [PATCH] src/fcgi-app.c: handle strdup failure

[Илья Шипицин]

updated patch attached (I preferred that instead of sending with "git
send-email")

пн, 5 авг. 2024 г. в 20:10, Илья Шипицин <chipits...@gmail.com>:

>
>
> пн, 5 авг. 2024 г. в 20:09, William Lallemand <wlallem...@irq6.net>:
>
>> On Mon, Aug 05, 2024 at 08:01:39PM +0200, Илья Шипицин wrote:
>> > Subject: Re: [PATCH] src/fcgi-app.c: handle strdup failure
>> > пн, 5 авг. 2024 г. в 19:56, William Lallemand <wlallem...@irq6.net>:
>> >
>> > > On Mon, Aug 05, 2024 at 07:17:48PM +0200, Ilia Shipitsin wrote:
>> > > > Subject: [PATCH] src/fcgi-app.c: handle strdup failure
>> > > > found by coccinelle
>> > >
>> > > Please add clearer commit messages in your patches, you tend to
>> minimize
>> > > them, thanks ! :-)
>> > >
>> >
>> > truth to be told, I tend to write longer messages than usually :)
>> >
>> >
>> > >
>> > > > ---
>> > > >  src/fcgi-app.c | 5 ++++-
>> > > >  1 file changed, 4 insertions(+), 1 deletion(-)
>> > > >
>> > > > diff --git a/src/fcgi-app.c b/src/fcgi-app.c
>> > > > index b3a9b7c59..d96bb222c 100644
>> > > > --- a/src/fcgi-app.c
>> > > > +++ b/src/fcgi-app.c
>> > > > @@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char
>> **args, int
>> > > section, struct proxy *curp
>> > > >       if (!fcgi_conf)
>> > > >               goto err;
>> > > >       fcgi_conf->name = strdup(args[1]);
>> > > > +     if (!fcgi_conf->name)
>> > > > +             goto err;
>> > > >       LIST_INIT(&fcgi_conf->param_rules);
>> > > >       LIST_INIT(&fcgi_conf->hdr_rules);
>> > > >
>> > > > @@ -622,7 +624,8 @@ static int proxy_parse_use_fcgi_app(char
>> **args, int
>> > > section, struct proxy *curp
>> > > >       return retval;
>> > > >    err:
>> > > >       if (fcgi_conf) {
>> > > > -             free(fcgi_conf->name);
>> > > > +             if (fcgi_conf->name)
>> > > > +                     free(fcgi_conf->name);
>> > >
>> > > You don't need to add a check there, free(NULL) does nothing.
>> > >
>> >
>> > I tried to figure out whether that behaviour is by chance or by purpose
>> > (taking into account variety of implementations on different OSes)
>> >
>> > I'll try again
>> >
>>
>> This is a standard behavior that is specified in POSIX, honestly HAProxy
>> won't probably run if it wasn't behaving this
>> way on some weird OS.
>>
>> https://pubs.opengroup.org/onlinepubs/009695399/functions/free.html "If
>> ptr is a null pointer, no action shall occur."
>>
>
> "man 3 free" didn't give me any link to POSIX.
> I've should have a look at POSIX directly, but I didn't
>
>
>>
>> --
>> William Lallemand
>>
>

From baa55f1434b2ed5288536f62b8e322ed20904b8d Mon Sep 17 00:00:00 2001
From: Ilia Shipitsin <chipits...@gmail.com>
Date: Mon, 5 Aug 2024 20:59:09 +0200
Subject: [PATCH] src/fcgi-app.c: handle strdup failure

this defect is found by the following coccinelle script:

// find calls to strdup
@call@
expression ptr;
position p;
@@

ptr@p = strdup(...);

// find ok calls to strdup
@ok@
expression ptr;
position call.p;
@@

ptr@p = strdup(...);
... when != ptr
(
 (ptr == NULL || ...)
|
 (ptr == 0 || ...)
|
 (ptr != NULL || ...)
|
 (ptr != 0 || ...)
)

// fix bad calls to strdup
@depends on !ok@
expression ptr;
position call.p;
@@

ptr@p = strdup(...);
+ if (ptr == NULL) return;
---
 src/fcgi-app.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/fcgi-app.c b/src/fcgi-app.c
index b3a9b7c59..98077b959 100644
--- a/src/fcgi-app.c
+++ b/src/fcgi-app.c
@@ -606,6 +606,8 @@ static int proxy_parse_use_fcgi_app(char **args, int 
section, struct proxy *curp
        if (!fcgi_conf)
                goto err;
        fcgi_conf->name = strdup(args[1]);
+       if (!fcgi_conf->name)
+               goto err;
        LIST_INIT(&fcgi_conf->param_rules);
        LIST_INIT(&fcgi_conf->hdr_rules);
 
-- 
2.43.0.windows.1