Skip site navigation (1)Skip section navigation (2) 
Date:      Sun, 24 Nov 2019 15:37:14 +0000 (UTC)
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r355057 - head/sys/dev/pci
Message-ID:  <201911241537.xAOFbEVF060099@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: imp
Date: Sun Nov 24 15:37:14 2019
New Revision: 355057
URL: https://svnweb.freebsd.org/changeset/base/355057

Log:
  Hoist locking giant back up into the ioctl handler
  
  Move the locking back into the ioctl handler. This "fixes" the race where we hve
  a hot plug event just after the dropping of Giant in pci_find_dbsf, assuming the
  driver doesn't then call anything that drops and picks up Giant again... It's a
  little safer since don't think it doesn't, but we lack the tools to know for
  sure.

Modified:
  head/sys/dev/pci/pci.c
  head/sys/dev/pci/pci_user.c

Modified: head/sys/dev/pci/pci.c
==============================================================================
--- head/sys/dev/pci/pci.c	Sun Nov 24 15:24:05 2019	(r355056)
+++ head/sys/dev/pci/pci.c	Sun Nov 24 15:37:14 2019	(r355057)
@@ -447,8 +447,6 @@ pci_find_dbsf(uint32_t domain, uint8_t bus, uint8_t sl
 {
 	struct pci_devinfo *dinfo = NULL;
 
-	/* Giant because newbus is Giant locked revisit with newbus locking */
-	mtx_lock(&Giant);
 	STAILQ_FOREACH(dinfo, &pci_devq, pci_links) {
 		if ((dinfo->cfg.domain == domain) &&
 		    (dinfo->cfg.bus == bus) &&
@@ -457,7 +455,6 @@ pci_find_dbsf(uint32_t domain, uint8_t bus, uint8_t sl
 			break;
 		}
 	}
-	mtx_unlock(&Giant);
 
 	return (dinfo != NULL ? dinfo->cfg.dev : NULL);
 }

Modified: head/sys/dev/pci/pci_user.c
==============================================================================
--- head/sys/dev/pci/pci_user.c	Sun Nov 24 15:24:05 2019	(r355056)
+++ head/sys/dev/pci/pci_user.c	Sun Nov 24 15:37:14 2019	(r355057)
@@ -965,6 +965,9 @@ pci_ioctl(struct cdev *dev, u_long cmd, caddr_t data, 
 	}
 
 
+	/* Giant because newbus is Giant locked revisit with newbus locking */
+	mtx_lock(&Giant);
+
 	switch (cmd) {
 	case PCIOCGETCONF:
 #ifdef COMPAT_FREEBSD32
@@ -1288,8 +1291,10 @@ getconfexit:
 	case PCIOCBARMMAP:
 		pbm = (struct pci_bar_mmap *)data;
 		if ((flag & FWRITE) == 0 &&
-		    (pbm->pbm_flags & PCIIO_BAR_MMAP_RW) != 0)
-			return (EPERM);
+		    (pbm->pbm_flags & PCIIO_BAR_MMAP_RW) != 0) {
+			error = EPERM;
+			break;
+		}
 		pcidev = pci_find_dbsf(pbm->pbm_sel.pc_domain,
 		    pbm->pbm_sel.pc_bus, pbm->pbm_sel.pc_dev,
 		    pbm->pbm_sel.pc_func);
@@ -1300,6 +1305,8 @@ getconfexit:
 		error = ENOTTY;
 		break;
 	}
+
+	mtx_unlock(&Giant);
 
 	return (error);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201911241537.xAOFbEVF060099>