linux-kernel - Re: [PATCH] can: slcan: fix freed work crash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221202035242.155d54f4.max@enpas.org>
Date:   Fri, 2 Dec 2022 03:52:42 +0900
From:   Max Staudt <max@...as.org>
To:     "Jiri Slaby (SUSE)" <jirislaby@...nel.org>
Cc:     dario.binacchi@...rulasolutions.com, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Richard Palethorpe <richard.palethorpe@...e.com>,
        Petr Vorel <petr.vorel@...e.com>,
        Wolfgang Grandegger <wg@...ndegger.com>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, linux-can@...r.kernel.org,
        netdev@...r.kernel.org, stable@...r.kernel.org, ltp@...ts.linux.it
Subject: Re: [PATCH] can: slcan: fix freed work crash

(CC: ltp@...ts.linux.it because Petr did so.)

Hi Jiry,

Thanks for finding this!

Your patch looks correct to me, so please have a

  Reviewed-by: Max Staudt <max@...as.org>

for both this patch to slcan, as well as an 1:1 patch to can327.

Some history:

This is actually my code from can327, which was backported to slcan as
part of Dario's larger modernisation effort.

The rationale for moving it was to flush the UART TX buffer in case of
ndo_close(), in order to bring the device into a more predictable state
between ndo_close() and ndo_open(). I guess that's actually
counterproductive - whatever is in the TX buffer at that time should
likely be fully sent. For example, can327 sends one last byte to abort
any running chatty monitoring mode from the adapter. So your patch also
fixes this as well ;)

Of course, this resulted in calling flush_worker() in both ndo_stop()
and ldisc_close(), so I wanted to avoid code duplication, and relied on
ndo_stop(). Oops.

Thanks,

Max