Hi Amaury,

Thanks for merging the first one, and for the rewrite. Your version suits
me, and the single check is cleaner. You are right about the allocation
failure, and about the backport going further back than 3.1. AF_NETLINK was
only how I reproduced it, so it is fine to leave it out of the message.

I applied your patch to current master (995295edc) and ran the same test
again. For the inherited netlink socket, "debug dev fd" now prints "laddr=?
raddr=?", and the process keeps running.

Thanks,
Martin

On Fri, Oct 30, 2026 06:24 PM, Amaury Denoyelle <[email protected]>
wrote:

> On Wed, Oct 28, 2026 at 07:46:44AM -0700, Martin Mirchev wrote:
> > Hi,
> > Here are the patches for GitHub issues #3545 and #3546, as Amaury
> > suggested in #3547, against current master (8c84ab562). They are attached
> > (git format-patch) and independent of each other.
> > 1/2 BUG/MINOR: sample: fix NULL dereference in the ltime/utime converters
> > conv_time_common() returns NULL when the expanded format does not fit
> > in the trash chunk or when alloc_trash_chunk() fails, and its four
> > callers dereference it. With ms_ltime("%N" x 3000) the config is
> > accepted and the first request crashes the process. The converters
> > now fail instead. Backport to 2.9 and later.
>
> Thanks, I merged this patch. I only slightly edited the commit message
> to correct the backport reference to 3.1 as this version is end of life.
>
> > 2/2 BUG/MINOR: debug: fix "debug dev fd" crash on unknown address
> families
> > sa2str() returns NULL for families other than AF_INET, AF_INET6 and
> > AF_UNIX, and "debug dev fd" passes it to strlen(). With an inherited
> > AF_NETLINK socket the command crashes the process. It now prints "?".
> > Backport to 3.1 and later.
> > Tested on 8c84ab562, release build (USE_OPENSSL=1): on master all four
> > converters (ms_ltime, us_ltime, ms_utime, us_utime) and "debug dev fd"
> with
> > an inherited netlink socket crash with SIGSEGV; with the patches each
> > converter request returns 200, "debug dev fd" prints "laddr=? raddr=?"
> for
> > the netlink fd, and the process keeps running.
> > Both issues were found with a static analysis engine (Infer/Pulse) and
> > verified manually.
>
> Regarding this patch, there is several points that can be improved. In
> my opinion the code is simpler with a single NULL check around both the
> cleanup loop and the dump. Also, the commit message is not totally
> accurate. First, I removed the reference to AF_NETLINK as I'm not sure
> haproxy is really dealing with such sockets. Then, sa2str() can also
> return NULL due to a memory allocation failure, so a crash could happen
> even with supported address families. Finally, regarding the backport, I
> think it should be picked up to 2.6, as the original "debug dev fd"
> handler already contains the dump of a possible NULL value.
>
> I have written a new version for the "debug dev fd" fix, attached as a
> patch with this email. Please tell me if this suits you.
>
> Thanks for your contribution,
>
> --
> Amaury Denoyelle
>

Reply via email to