lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 5 Jan 2018 11:44:38 +0800 From: "jianchao.wang" <jianchao.w.wang@...cle.com> To: Christoph Hellwig <hch@....de> Cc: keith.busch@...el.com, axboe@...com, sagi@...mberg.me, linux-nvme@...ts.infradead.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH V2] nvme-pci: fix NULL pointer reference in nvme_alloc_ns Hi Christoph Many thanks for your kindly response. On 01/04/2018 06:20 PM, Christoph Hellwig wrote: > This looks generally fine to me, ut a few nitpicks below: > >> - Based on Sagi's suggestion, add new state NVME_CTRL_ADMIN_LIVE. > > Maybe call this NVME_CTRL_ADMIN_ONLY ? Sound more in line with the new state. Use it in next version. > >> - if (ctrl->state != NVME_CTRL_LIVE) >> + if ((ctrl->state != NVME_CTRL_LIVE) && >> + (ctrl->state != NVME_CTRL_ADMIN_LIVE)) > > No need for the inner braces, and odd indentation. Also in general > I'm tempted to just use switch statements for things like this, e.g. > > switch (ctrl->state) { > case NVME_CTRL_ADMIN_LIVE: > case NVME_CTRL_LIVE: > break; > default: > return -EWOULDBLOCK; > } > Yes, it looks clearer and more readable. Use it in next version >> @@ -3074,6 +3087,8 @@ static void nvme_scan_work(struct work_struct *work) >> if (ctrl->state != NVME_CTRL_LIVE) >> return; >> >> + BUG_ON(!ctrl->tagset); > > WARN_ON_ONCE() please. Yes, use it in next version > >> + bool only_adminq = false; > > How about a new_state variable instead that holds the new state value? > Yes, it is more reasonable. Use it next version. Thanks Jianchao
Powered by blists - more mailing lists