Defend against calling sysctl_ctx_free on uninitialized context

Kenneth D. Merry ken at kdm.org
Tue Oct 7 06:00:44 PDT 2003


On Fri, Sep 19, 2003 at 19:50:32 +0200, Thomas Quinot wrote:
> If a fatal error occurs while cd(4) is attaching, before the sysctl_ctx
> has been initialized, then it must not be freed. The following patch
> resolves this problem, please review.
> 
> Thanks,
> Thomas.
> 
> Index: scsi_cd.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/cam/scsi/scsi_cd.c,v
> retrieving revision 1.83
> diff -u -r1.83 scsi_cd.c
> --- scsi_cd.c	11 Sep 2003 19:27:24 -0000	1.83
> +++ scsi_cd.c	19 Sep 2003 17:47:03 -0000
> @@ -91,17 +91,18 @@
>  } cd_quirks;
>  
>  typedef enum {
> -	CD_FLAG_INVALID		= 0x001,
> -	CD_FLAG_NEW_DISC	= 0x002,
> -	CD_FLAG_DISC_LOCKED	= 0x004,
> -	CD_FLAG_DISC_REMOVABLE	= 0x008,
> -	CD_FLAG_TAGGED_QUEUING	= 0x010,
> -	CD_FLAG_CHANGER		= 0x040,
> -	CD_FLAG_ACTIVE		= 0x080,
> -	CD_FLAG_SCHED_ON_COMP	= 0x100,
> -	CD_FLAG_RETRY_UA	= 0x200,
> -	CD_FLAG_VALID_MEDIA	= 0x400,
> -	CD_FLAG_VALID_TOC	= 0x800
> +	CD_FLAG_INVALID		= 0x0001,
> +	CD_FLAG_NEW_DISC	= 0x0002,
> +	CD_FLAG_DISC_LOCKED	= 0x0004,
> +	CD_FLAG_DISC_REMOVABLE	= 0x0008,
> +	CD_FLAG_TAGGED_QUEUING	= 0x0010,
> +	CD_FLAG_CHANGER		= 0x0040,
> +	CD_FLAG_ACTIVE		= 0x0080,
> +	CD_FLAG_SCHED_ON_COMP	= 0x0100,
> +	CD_FLAG_RETRY_UA	= 0x0200,
> +	CD_FLAG_VALID_MEDIA	= 0x0400,
> +	CD_FLAG_VALID_TOC	= 0x0800,
> +	CD_FLAG_SCTX_INIT	= 0x1000
>  } cd_flags;
>  
>  typedef enum {
> @@ -458,7 +459,8 @@
>  	xpt_print_path(periph->path);
>  	printf("removing device entry\n");
>  
> -	if (sysctl_ctx_free(&softc->sysctl_ctx) != 0) {
> +	if ((softc->flags & CD_FLAG_SCTX_INIT) != 0
> +	    && sysctl_ctx_free(&softc->sysctl_ctx) != 0) {
>  		xpt_print_path(periph->path);
>  		printf("can't remove sysctl context\n");
>  	}
> @@ -622,6 +624,7 @@
>  	mtx_lock(&Giant);
>  
>  	sysctl_ctx_init(&softc->sysctl_ctx);
> +	softc->flags |= CD_FLAG_SCTX_INIT;
>  	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
>  		SYSCTL_STATIC_CHILDREN(_kern_cam_cd), OID_AUTO,
>  		tmpstr2, CTLFLAG_RD, 0, tmpstr);
> 
> -- 
>     Thomas.Quinot at Cuivre.FR.EU.ORG

This looks fine, feel free to commit.

There is also a potential race condition between the device going away and
the taskqueue running.  If the device goes away before the taskqueue runs,
it'll end up shoving sysctl context into freed memory, and will likely
leave the sysctl variable for that cd(4) instance hanging around.

It doesn't look like there is a taskqueue_dequeue(), though, so we can't
check for an active taskqueue request in cdcleanup() and dequeue it.

Ken
-- 
Kenneth Merry
ken at kdm.org


More information about the freebsd-scsi mailing list