Date: Sun, 18 Jun 2017 19:41:20 +0800 From: Jia-Ju Bai <baijiaju1990@163.com> To: rkoberman@gmail.com, yongari@freebsd.org Cc: freebsd-drivers@freebsd.org, freebsd-net@freebsd.org Subject: Re: [Bug 220032][PATCH] if_alc: Fix possible sleep-under-mutex bugs Message-ID: <069fa517-2774-c19b-f2c5-a3b81df1a812@163.com> In-Reply-To: <20170618010405.40107-1-baijiaju1990@163.com> References: <20170618010405.40107-1-baijiaju1990@163.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
I have read the manual page of bus_dmamap_load_mbuf_sg.
This call will always return immediately and will not block for any reason.
Sorry for my wrong report, please ignore it.
Thanks,
Jia-Ju Bai
On 2017/6/18 9:04, Jia-Ju Bai wrote:
> The alc driver may sleep under a mutex, and the function call paths in file
> "sys/dev/alc/if_alc.c" are:
> alc_resume [acquire the mutex]
> alc_init_locked
> alc_init_rx_ring
> alc_newbuf
> bus_dmamap_load_mbuf_sg(BUS_DMA_WAITOK) --> may sleep
> alc_start [acquire the mutex]
> alc_start_locked
> alc_encap
> bus_dmamap_load_mbuf_sg(BUS_DMA_WAITOK) --> may sleep
>
> The possible fix of these bugs is to set the last parameter in
> bus_dmamap_load_mbuf_sg to "BUS_DMA_NOWAIT".
>
> This bug is found by a static analysis tool written by myself, and it is
> checked by my review of the FreeBSD code.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
> ---
> sys/dev/alc/if_alc.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/sys/dev/alc/if_alc.c b/sys/dev/alc/if_alc.c
> index ca7ae9d17b5..cb0f15e223b 100644
> --- a/sys/dev/alc/if_alc.c
> +++ b/sys/dev/alc/if_alc.c
> @@ -2795,7 +2795,7 @@ alc_encap(struct alc_softc *sc, struct mbuf **m_head)
> map = txd->tx_dmamap;
>
> error = bus_dmamap_load_mbuf_sg(sc->alc_cdata.alc_tx_tag, map,
> - *m_head, txsegs, &nsegs, 0);
> + *m_head, txsegs, &nsegs, BUS_DMA_NOWAIT);
> if (error == EFBIG) {
> m = m_collapse(*m_head, M_NOWAIT, ALC_MAXTXSEGS);
> if (m == NULL) {
> @@ -2805,7 +2805,7 @@ alc_encap(struct alc_softc *sc, struct mbuf **m_head)
> }
> *m_head = m;
> error = bus_dmamap_load_mbuf_sg(sc->alc_cdata.alc_tx_tag, map,
> - *m_head, txsegs, &nsegs, 0);
> + *m_head, txsegs, &nsegs, BUS_DMA_NOWAIT);
> if (error != 0) {
> m_freem(*m_head);
> *m_head = NULL;
> @@ -3487,7 +3487,7 @@ alc_newbuf(struct alc_softc *sc, struct alc_rxdesc *rxd)
> #endif
>
> if (bus_dmamap_load_mbuf_sg(sc->alc_cdata.alc_rx_tag,
> - sc->alc_cdata.alc_rx_sparemap, m, segs, &nsegs, 0) != 0) {
> + sc->alc_cdata.alc_rx_sparemap, m, segs, &nsegs, BUS_DMA_NOWAIT) != 0) {
> m_freem(m);
> return (ENOBUFS);
> }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?069fa517-2774-c19b-f2c5-a3b81df1a812>