d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a panic in camperiphfree().

Browse Source 
If a peripheral driver (e.g. da, sa, cd) is added or removed from the
peripheral driver list while an unrelated peripheral driver instance (e.g.
da0, sa5, cd2) is going away and is inside camperiphfree(), we could
dereference an invalid pointer.

When peripheral drivers are added or removed (see periphdriver_register()
and periphdriver_unregister()), the peripheral driver array is resized
and existing entries are moved.

Although we hold the topology lock while we traverse the peripheral driver
list, we retain a pointer to the location of the peripheral driver pointer
and then drop the topology lock.  So we are still vulnerable to the list
getting moved around while the lock is dropped.

To solve the problem, cache a copy of the peripheral driver pointer.  If
its storage location in the list changes while we have the lock dropped, it
won't have any effect.

This doesn't solve the issue that peripheral drivers ("da", "cd", as opposed
to individual instances like "da0", "cd0") are not generally part of a
reference counting scheme to guard against deregistering them while there
are instances active.  The caller (generally the person unloading a module)
has to be aware of active drivers and not unload something that is in use.

sys/cam/cam_periph.c:
	In camperiphfree(), cache a pointer to the peripheral driver
	instance to avoid holding a pointer to an invalid memory location
	in the event that the peripheral driver list changes while we have
	the topology lock dropped.

PR:		kern/219701
Submitted by:	avg
MFC after:	3 days
Sponsored by:	Spectra Logic
This commit is contained in:
Kenneth D. Merry 2017-06-27 19:26:02 +00:00
parent 89763b3f8e
commit 59fe76647c
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=320421
1 changed files with 12 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/cam/cam_periph.c
View File

@ -661,6 +661,7 @@ static void
camperiphfree(struct cam_periph *periph)
{
struct periph_driver **p_drv;
struct periph_driver *drv;
cam_periph_assert(periph, MA_OWNED);
KASSERT(periph->periph_allocating == 0, ("%s%d: freed while allocating",
@ -673,6 +674,15 @@ camperiphfree(struct cam_periph *periph)
printf("camperiphfree: attempt to free non-existant periph\n");
return;
}
/*
* Cache a pointer to the periph_driver structure. If a
* periph_driver is added or removed from the array (see
* periphdriver_register()) while we drop the toplogy lock
* below, p_drv may change. This doesn't protect against this
* particular periph_driver going away. That will require full
* reference counting in the periph_driver infrastructure.
*/
drv = *p_drv;
/*
* We need to set this flag before dropping the topology lock, to
@ -708,8 +718,8 @@ camperiphfree(struct cam_periph *periph)
*/
xpt_lock_buses();
TAILQ_REMOVE(&(*p_drv)->units, periph, unit_links);
(*p_drv)->generation++;
TAILQ_REMOVE(&drv->units, periph, unit_links);
drv->generation++;
xpt_remove_periph(periph);