Bug 215356

Summary: sysutils/smartmontools: fix panic on INVARIANTS enabled kernel after r308351 in base
Product: Ports & Packages Reporter: op
Component: Individual Port(s)Assignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Some People CC: asomers, avinkon, ken, markj, mav, samm, tijl, w.schwarzenfeld
Priority: --- Keywords: patch
Version: LatestFlags: samm: maintainer-feedback+
op: merge-quarterly?
Hardware: Any   
OS: Any   
Attachments:
Description Flags
0001-smartmontools-fix-panic-on-INVARIANTS-enabled-kernel.patch
none
picture from panic none

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 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 2017-01-08 23:11:31 UTC
Thank you for your patch, please proceed, accepted
Comment 7 Tijl Coosemans freebsd_committer 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 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 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 2017-02-07 15:58:03 UTC
Assign to markj for the kernel side.
Comment 11 commit-hook freebsd_committer 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 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 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