From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net-next] ptr_ring: fix integer overflow Date: Fri, 26 Jan 2018 11:44:22 +0800 Message-ID: <29dd0d30-07e7-0432-2ad8-209a2ed35e5a@redhat.com> References: <1516865502-20835-1-git-send-email-jasowang@redhat.com> <20180125154255-mutt-send-email-mst@kernel.org> <81ecef6f-5076-873c-2f0d-e08e0a35dcf5@redhat.com> <20180125193131-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org, John Fastabend To: "Michael S. Tsirkin" Return-path: In-Reply-To: <20180125193131-mutt-send-email-mst@kernel.org> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On 2018年01月26日 01:31, Michael S. Tsirkin wrote: > On Thu, Jan 25, 2018 at 10:17:38PM +0800, Jason Wang wrote: >> >> On 2018年01月25日 21:45, Michael S. Tsirkin wrote: >>> On Thu, Jan 25, 2018 at 03:31:42PM +0800, Jason Wang wrote: >>>> We try to allocate one more entry for lockless peeking. The adding >>>> operation may overflow which causes zero to be passed to kmalloc(). >>>> In this case, it returns ZERO_SIZE_PTR without any notice by ptr >>>> ring. Try to do producing or consuming on such ring will lead NULL >>>> dereference. Fix this detect and fail early. >>>> >>>> Fixes: bcecb4bbf88a ("net: ptr_ring: otherwise safe empty checks can overrun array bounds") >>>> Reported-by:syzbot+87678bcf753b44c39b67@syzkaller.appspotmail.com >>>> Cc: John Fastabend >>>> Signed-off-by: Jason Wang >>> Ugh that's just way too ugly. >>> I'll work on dropping the extra + 1 - but calling this >>> function with -1 size is the real source of the bug. >>> Do you know how come we do that? >>> >> It looks e.g try to change tx_queue_len to UINT_MAX. And we probably can't >> prevent user form trying to do this? >> >> Thanks > Right. BTW why net-next? Isn't the crash exploitable in net? > Commit bcecb4bbf88a exists only in net-next. And in net we check r->size before trying to dereference the queue. Thanks