NetBSD Problem Report #52900

From www@NetBSD.org  Fri Jan  5 13:38:51 2018
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 1F3F77A1B2
	for <gnats-bugs@gnats.NetBSD.org>; Fri,  5 Jan 2018 13:38:51 +0000 (UTC)
Message-Id: <20180105133849.D63877A220@mollari.NetBSD.org>
Date: Fri,  5 Jan 2018 13:38:49 +0000 (UTC)
From: kusumi.tomohiro@gmail.com
Reply-To: kusumi.tomohiro@gmail.com
To: gnats-bugs@NetBSD.org
Subject: dm: Fix kernel panic when target's ->init() failed
X-Send-Pr-Version: www-1.0

>Number:         52900
>Category:       kern
>Synopsis:       dm: Fix kernel panic when target's ->init() failed
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jan 05 13:40:00 +0000 2018
>Closed-Date:    Fri Feb 09 18:56:58 +0000 2018
>Last-Modified:  Fri Feb 09 18:56:58 +0000 2018
>Originator:     Tomohiro Kusumi
>Release:        -current, but existed since before.
>Organization:
>Environment:
Architecture independent.
# uname -r
8.99.9

>Description:
Having dm_target_unbusy() when target's ->init() failed causes
kernel panic if DIAGNOSTIC is enabled at L65 of below function,
and it's wrong anyway even if DIAGNOSTIC isn't enabled.

If ->init() failed, ->destroy() called by dm_table_destroy() has
already called dm_target_unbusy(), thus it shouldn't be calling
it again to decrement ->ref_cnt to below zero. It's either ioctl
side or ->destroy() who unrefs the counter, but not both.

 62 void
 63 dm_target_unbusy(dm_target_t * target)
 64 {
 65         KASSERT(target->ref_cnt > 0);
 66         atomic_dec_32(&target->ref_cnt);
 67 }

>How-To-Repeat:
The screenshot in the below link is an example of what happens
when ->init() handler fails (i.e. returns non zero) with DIAGNOSTIC
enabled. Having dm_target_unbusy() here means dm target drivers
can't fail on ->init() otherwise wrongly unref or panic.

https://leaf.dragonflybsd.org/~tkusumi/diff/netbsd/dm_ref_cnt_panic.PNG

>Fix:
https://leaf.dragonflybsd.org/~tkusumi/diff/netbsd/dm-1.patch

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52900 CVS commit: src/sys/dev/dm
Date: Fri, 5 Jan 2018 09:22:05 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Fri Jan  5 14:22:05 UTC 2018

 Modified Files:
 	src/sys/dev/dm: dm_ioctl.c dm_target_linear.c dm_target_snapshot.c

 Log Message:
 PR/52900: Tomohiro Kusumi: Fix kernel panic when target's ->init() failed
 Reliably unbusy the target in the *all* the destroy routines, so ioctl
 does not need to do it.


 To generate a diff of this commit:
 cvs rdiff -u -r1.31 -r1.32 src/sys/dev/dm/dm_ioctl.c
 cvs rdiff -u -r1.16 -r1.17 src/sys/dev/dm/dm_target_linear.c
 cvs rdiff -u -r1.18 -r1.19 src/sys/dev/dm/dm_target_snapshot.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: maxv@NetBSD.org
State-Changed-When: Fri, 09 Feb 2018 18:56:58 +0000
State-Changed-Why:
The issue was fixed, thanks for the report.


>Unformatted:

Home	PR Database Search