[Bug 220032][PATCH] if_alc: Fix possible sleep-under-mutex bugs

[Jia-Ju Bai]

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 at 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);
>   	}