Bug 215356 - sysutils/smartmontools: fix panic on INVARIANTS enabled kernel after r308351 in base
Summary: sysutils/smartmontools: fix panic on INVARIANTS enabled kernel after r308351...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Mark Johnston
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2016-12-17 19:34 UTC by op
Modified: 2018-03-14 20:17 UTC (History)
8 users (show)

See Also:
samm: maintainer-feedback+
op: merge-quarterly?

Attachments
0001-smartmontools-fix-panic-on-INVARIANTS-enabled-kernel.patch (1.54 KB, patch)
2016-12-17 19:34 UTC, op 		no flags Details | Diff
picture from panic (286.22 KB, image/jpeg)
2016-12-17 19:40 UTC, op 		no flags Details
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description op 2016-12-17 19:34:38 UTC 
Created attachment 178032 [details]
0001-smartmontools-fix-panic-on-INVARIANTS-enabled-kernel.patch

smartmontools: fix panic on INVARIANTS enabled kernel after r308351 in base
    
    After r308351 commit the INVARIANTS enabled kernel gets too "eager" when
    checking the cam ccb flags. But the real problem was not the previous, but
    the smartmontools does not zeroed out the ccb on stack, and pass this
    dirty ccb to kernel, which is catched by an ASSERT. So fix this by filling
    up with 0 this ccb. This commit is similar, what's happen in r307684
    in camcontrol. So this is the same followup patch in smartmontools.
    
    https://svnweb.freebsd.org/base?view=revision&revision=307684
    https://svnweb.freebsd.org/base?view=revision&revision=308351
    
    Sponsored-by: opBSD
Comment 1 op 2016-12-17 19:40:18 UTC 
Created attachment 178034 [details]
picture from panic
Comment 2 op 2016-12-17 19:41:55 UTC 
How to reproduce:

1) add drives to smartd.conf

cat > /usr/local/etc.smartd.conf<<EOF
/dev/ada0
/dev/ada1
EOF

2) restart some times the smartd

repeat 100 service smartd restart

3) wait for panic
Comment 3 Alan Somers freebsd_committer freebsd_triage 2016-12-17 19:56:42 UTC 
Sounds like an upstream bug.  Have you reported it to smartmontools?
Comment 4 op 2016-12-17 19:58:07 UTC 
Not yet, I have found this error an hour ago, but it's on my todo list.
Comment 6 Oleksii Samorukov freebsd_committer freebsd_triage 2017-01-08 23:11:31 UTC 
Thank you for your patch, please proceed, accepted
Comment 7 Tijl Coosemans freebsd_committer freebsd_triage 2017-01-10 11:45:03 UTC 
markj/mav, can you take a look at base r306529?  The kasserts added there can be triggered to panic from userland.
Comment 8 Mark Johnston freebsd_committer freebsd_triage 2017-01-10 21:24:55 UTC 
(In reply to Tijl Coosemans from comment #7)
Hm, it seems that passdoioctl() should really be sanitizing the CCB flags. At least CAM_UNLOCKED cannot correctly be specified by userland.
Comment 9 Oleksii Samorukov freebsd_committer freebsd_triage 2017-01-29 21:08:35 UTC 
Hi, why the status is still "maintainer feedback"? I approved this change long time ago, please commit.
Comment 10 Tijl Coosemans freebsd_committer freebsd_triage 2017-02-07 15:58:03 UTC 
Assign to markj for the kernel side.
Comment 11 commit-hook freebsd_committer freebsd_triage 2017-03-03 20:52:34 UTC 
A commit references this bug:

Author: markj
Date: Fri Mar  3 20:51:57 UTC 2017
New revision: 314624
URL: https://svnweb.freebsd.org/changeset/base/314624

Log:
  Reject userland CCBs that have CAM_UNLOCKED set.

  CAM_UNLOCKED is internal flag and cannot correctly be set by userland.
  Return EINVAL from CAMIOCOMMAND and CAMIOQUEUE if it is set.

  Also fix leaks in some of the error paths for CAMIOQUEUE.

  PR:		215356
  Reviewed by:	ken, mav
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D9869

Changes:
  head/sys/cam/cam_xpt.c
  head/sys/cam/scsi/scsi_pass.c
Comment 12 Walter Schwarzenfeld freebsd_triage 2018-03-04 08:31:41 UTC 
Forgotten to close?
Comment 13 Tijl Coosemans freebsd_committer freebsd_triage 2018-03-04 10:48:22 UTC 
The attached patch has been committed upstream and the kernel side has been fixed as well.
Comment 14 Avinash K 2018-03-13 22:12:18 UTC 
Iam hitting this bug easily in stable_11 , at the time of boot itself with smartd.conf having following field DEVICESCAN . Iam not seeing the commit yet in stable_11
Comment 15 commit-hook freebsd_committer freebsd_triage 2018-03-14 09:58:59 UTC 
A commit references this bug:

Author: tijl
Date: Wed Mar 14 09:57:58 UTC 2018
New revision: 330926
URL: https://svnweb.freebsd.org/changeset/base/330926

Log:
  MFC r314624:

  Reject userland CCBs that have CAM_UNLOCKED set.

  CAM_UNLOCKED is internal flag and cannot correctly be set by userland.
  Return EINVAL from CAMIOCOMMAND and CAMIOQUEUE if it is set.

  Also fix leaks in some of the error paths for CAMIOQUEUE.

  PR:		215356

Changes:
_U  stable/11/
  stable/11/sys/cam/cam_xpt.c
  stable/11/sys/cam/scsi/scsi_pass.c
Comment 16 Avinash K 2018-03-14 20:17:54 UTC 
(In reply to commit-hook from comment #15)
Thank you tijl for your quick response , I see the fix now and its working for me