From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756636AbcGGJmc (ORCPT ); Thu, 7 Jul 2016 05:42:32 -0400 Received: from prv-mh.provo.novell.com ([137.65.248.74]:60500 "EHLO prv-mh.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751691AbcGGJma convert rfc822-to-8bit (ORCPT ); Thu, 7 Jul 2016 05:42:30 -0400 Message-Id: <577E400A02000078000FC0CB@prv-mh.provo.novell.com> X-Mailer: Novell GroupWise Internet Agent 14.2.0 Date: Thu, 07 Jul 2016 03:42:02 -0600 From: "Jan Beulich" To: "Roger Pau Monne" Cc: , "Konrad Rzeszutek Wilk" , Subject: Re: [PATCH] xen-blkfront: avoid NULL de-reference in CDROM ioctl handling References: <577E23A602000078000FBE15@prv-mh.provo.novell.com> <20160707093211.dn5mrvgmavpd2mul@mac> In-Reply-To: <20160707093211.dn5mrvgmavpd2mul@mac> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8BIT Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >>> On 07.07.16 at 11:32, wrote: > On Thu, Jul 07, 2016 at 01:40:54AM -0600, Jan Beulich wrote: >> The ioctl can be called prior to full device setup having completed. >> >> Signed-off-by: Jan Beulich >> --- >> drivers/block/xen-blkfront.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> --- 4.7-rc6-xen.orig/drivers/block/xen-blkfront.c >> +++ 4.7-rc6-xen/drivers/block/xen-blkfront.c >> @@ -496,12 +496,10 @@ static int blkif_ioctl(struct block_devi >> return -EFAULT; >> return 0; >> >> - case CDROM_GET_CAPABILITY: { >> - struct gendisk *gd = info->gd; >> - if (gd->flags & GENHD_FL_CD) >> + case CDROM_GET_CAPABILITY: >> + if (info->gd && (info->gd->flags & GENHD_FL_CD)) > > I don't follow how blkif_ioctl can be called with a NULL info->gd, because > the set of file operations is set inside of info->gd->fops. And the disk > should not be available until add_disk is called, which happens after having > info->gd already set. Well, this (as many of the other patches sent earlier today) is a result of me finally doing a sweep over the old 2.6.18 tree to see which changes never made it upstream. This is one of them. Hence I can't entirely exclude that the issue cannot be observed on a modern kernel (for whatever reason), but it surely was observed years ago on that old kernel. The sequence of events in xlvbd_alloc_gendisk() alone certainly leaves room for this, but I agree that the call to add_disk() _should_ be a prereq to getting into blkif_ioctl() and definitely only happens after xlvbd_alloc_gendisk() completed. But I guess you agree that this _should_ should also have applied to 2.6.18. Jan