From owner-freebsd-drivers@freebsd.org Sun Jun 18 11:41:21 2017 Return-Path: Delivered-To: freebsd-drivers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id CA7E1D8EE3A; Sun, 18 Jun 2017 11:41:21 +0000 (UTC) (envelope-from baijiaju1990@163.com) Received: from m12-12.163.com (m12-12.163.com [220.181.12.12]) by mx1.freebsd.org (Postfix) with ESMTP id E8DC5688A8; Sun, 18 Jun 2017 11:41:20 +0000 (UTC) (envelope-from baijiaju1990@163.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Subject:From:Message-ID:Date:MIME-Version; bh=DZ803 I9acetEcBgiGE6FV3e2e2nRNm5qaB6FMNtp54c=; b=Wq9DDOHkn4T5qxQsY63ws ToC8b/8XUGS1RC1xx1txEvnDlPX0rpm4+gZc1diJTw8N4LSEhMsiQXTpmHgE4MRF 2hX2c+XplNObSMz0sJ/o/I3Vu4/gt//xHb1ur7VUuIY/scLjCfSygwm1oTlWl+rx BTnzZlfAaRXhl9F1Rdnp7A= Received: from [166.111.70.13] (unknown [166.111.70.13]) by smtp8 (Coremail) with SMTP id DMCowABXg6ffZkZZQUQEDA--.35208S2; Sun, 18 Jun 2017 19:41:19 +0800 (CST) Subject: Re: [Bug 220032][PATCH] if_alc: Fix possible sleep-under-mutex bugs To: rkoberman@gmail.com, yongari@freebsd.org Cc: freebsd-drivers@freebsd.org, freebsd-net@freebsd.org References: <20170618010405.40107-1-baijiaju1990@163.com> From: Jia-Ju Bai Message-ID: <069fa517-2774-c19b-f2c5-a3b81df1a812@163.com> Date: Sun, 18 Jun 2017 19:41:20 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170618010405.40107-1-baijiaju1990@163.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-CM-TRANSID: DMCowABXg6ffZkZZQUQEDA--.35208S2 X-Coremail-Antispam: 1Uf129KBjvJXoW7ZrW7XF43ArWDAr18ZF48Xrb_yoW8uw43pa y7WFy5uryYyw40va48KF40g3W8t34rZry5GrW8Cr93Grn8Gr1rW3yUAa1fZF4a9rZ7CFyf Xry5u3s8KrWUAFDanT9S1TB71UUUUUUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07jQ0edUUUUU= X-Originating-IP: [166.111.70.13] X-CM-SenderInfo: xedlyx5dmximizq6il2tof0z/1tbiTQD6elc69sIE9QAAsl X-BeenThere: freebsd-drivers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Writing device drivers for FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jun 2017 11:41:21 -0000 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 > --- > 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); > }