Date: Mon, 17 May 2021 09:52:45 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: Warner Losh <imp@bsdimp.com>, Mark Johnston <markj@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: git: 0f206cc91279 - main - cam: add missing zeroing of a stack-allocated CCB. Message-ID: <YKISvdE3jVAAVXUF@kib.kiev.ua> In-Reply-To: <YKFebPTnf1jCZtkD@brick> References: <202105161045.14GAjZIL093217@gitrepo.freebsd.org> <YKFOeDlJhm8sJHJX@nuc> <CANCZdfrPuWJeH4xK6Z0E1c2s6fvn5_tcwd0wN%2Bs7R-aKD61zqQ@mail.gmail.com> <YKFebPTnf1jCZtkD@brick>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, May 16, 2021 at 07:03:24PM +0100, Edward Tomasz Napierala wrote: > On 0516T1227, Warner Losh wrote: > > On Sun, May 16, 2021, 11:55 AM Mark Johnston <markj@freebsd.org> wrote: > > > > > On Sun, May 16, 2021 at 10:45:35AM +0000, Edward Tomasz Napierala wrote: > > > > The branch main has been updated by trasz: > > > > > > > > URL: > > > https://cgit.FreeBSD.org/src/commit/?id=0f206cc91279e630ad9e733eb6e330b7dbe6c70e > > > > > > > > commit 0f206cc91279e630ad9e733eb6e330b7dbe6c70e > > > > Author: Edward Tomasz Napierala <trasz@FreeBSD.org> > > > > AuthorDate: 2021-05-16 09:28:04 +0000 > > > > Commit: Edward Tomasz Napierala <trasz@FreeBSD.org> > > > > CommitDate: 2021-05-16 10:38:26 +0000 > > > > > > > > cam: add missing zeroing of a stack-allocated CCB. > > > > > > > > This could cause a panic at boot. > > > > > > There are other instances of this, for example syzbot is currently > > > hitting an assertion, seemingly because the alloc_flags field of a > > > stack-allocated CCB was not zeroed: > > > https://syzkaller.appspot.com/bug?extid=2e9ce63919709feb3d1c > > > > > > I think the patch below will fix it, but I did not audit other callers. > > > It feels a bit strange to require all callers of xpt_setup_ccb() to > > > manually zero the structure first, can we provide a single routine to > > > initialize stack-allocated CCBs? > > We definitely could, although in some cases it's a bit more > complicated than that - a function that gets passed a CCB and then > calls xpt_setup_ccb() to fill it shouldn't zero it, as that would > be making assumption on how the CCB passed to it was allocated. > > Now that I look at the code, I can definitely see that I've missed > a couple of places. Perhaps I should replace those two KASSERTs with > diagnostic printfs for now, until I get that sorted out? > > > If we did, we could set a flag we could assert on, and/or do static > > analysis to find any others... > > That sounds promising, except I've never done anything like that; > I don't even know where to start. Are stack-allocated ccbs passed around? In particular, can the thread that allocated the ccb, put to sleep while ccb is processed elsewere? This should break under swapping.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YKISvdE3jVAAVXUF>