<feed xmlns='http://www.w3.org/2005/Atom'>
<title>src/sys/dev/firewire/sbp.c, branch releng/12.2</title>
<subtitle>FreeBSD source tree</subtitle>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/'/>
<entry>
<title>sys/dev: further adoption of SPDX licensing ID tags.</title>
<updated>2017-11-27T14:52:40+00:00</updated>
<author>
<name>Pedro F. Giffuni</name>
<email>pfg@FreeBSD.org</email>
</author>
<published>2017-11-27T14:52:40+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=718cf2ccb9956613756ab15d7a0e28f2c8e91cab'/>
<id>718cf2ccb9956613756ab15d7a0e28f2c8e91cab</id>
<content type='text'>
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.
</pre>
</div>
</content>
</entry>
<entry>
<title>firewire/sbp: try to improve locking, plus a few style nits</title>
<updated>2017-03-07T16:07:52+00:00</updated>
<author>
<name>Andriy Gapon</name>
<email>avg@FreeBSD.org</email>
</author>
<published>2017-03-07T16:07:52+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=857bb3d01e9a4b357dec4ea6c47f085d2dca064c'/>
<id>857bb3d01e9a4b357dec4ea6c47f085d2dca064c</id>
<content type='text'>
This change tries to fix the most obvious locking problems.

sbp_cam_scan_lun() is never called with the sbp lock held, so the lock
needs to be acquired internally (if it's needed at all).
Without this change a kernel with INVARIANTS panics when a firewire disk
is connected:
  panic: mutex sbp not owned at /usr/src/sys/dev/firewire/sbp.c:967
  KDB: stack backtrace:
  db_trace_self_wrapper() at 0xffffffff80420bbb = db_trace_self_wrapper+0x2b/frame 0xfffffe0504df0930
  kdb_backtrace() at 0xffffffff80670359 = kdb_backtrace+0x39/frame 0xfffffe0504df09e0
  vpanic() at 0xffffffff8063986c = vpanic+0x14c/frame 0xfffffe0504df0a20
  panic() at 0xffffffff806395b3 = panic+0x43/frame 0xfffffe0504df0a80
  __mtx_assert() at 0xffffffff8061c40d = __mtx_assert+0xed/frame 0xfffffe0504df0ac0
  sbp_cam_scan_lun() at 0xffffffff80474667 = sbp_cam_scan_lun+0x37/frame 0xfffffe0504df0af0
  xpt_done_process() at 0xffffffff802aacfa = xpt_done_process+0x2da/frame 0xfffffe0504df0b30
  xpt_done_td() at 0xffffffff802ac2e5 = xpt_done_td+0xd5/frame 0xfffffe0504df0b80
  fork_exit() at 0xffffffff805ff72f = fork_exit+0xdf/frame 0xfffffe0504df0bf0
  fork_trampoline() at 0xffffffff8082483e = fork_trampoline+0xe/frame
  0xfffffe0504df0bf0
  --- trap 0, rip = 0, rsp = 0, rbp = 0 ---

Also, I tried to reduce the scope of the sbp lock to avoid holding it
while doing bus_dma allocations.

The code badly needs some re-engineering.  SBP really should implement
a CAM transport, so that it avoids control flow inversion when re-scanning
the bus.  Also, the sbp lock seems to be too coarse.

Additionally, the commit includes some changes not related to locking.

- sbp_cam_scan_lun: restore CAM_DEV_QFREEZE before re-queueing the ccb
  because xpt_setup_ccb resets ccb_h.flags
- sbp_post_busreset: call xpt_release_simq only if it's actually frozen
- don't place private SIMQ_FREEZED flag (sic, "freezed") into sim-&gt;flags,
  use sbp-&gt;flags for that
- some style fixes and control flow enhancements

Reviewed by:	sbruno
MFC after:	2 weeks
Differential Revision: https://reviews.freebsd.org/D9898
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This change tries to fix the most obvious locking problems.

sbp_cam_scan_lun() is never called with the sbp lock held, so the lock
needs to be acquired internally (if it's needed at all).
Without this change a kernel with INVARIANTS panics when a firewire disk
is connected:
  panic: mutex sbp not owned at /usr/src/sys/dev/firewire/sbp.c:967
  KDB: stack backtrace:
  db_trace_self_wrapper() at 0xffffffff80420bbb = db_trace_self_wrapper+0x2b/frame 0xfffffe0504df0930
  kdb_backtrace() at 0xffffffff80670359 = kdb_backtrace+0x39/frame 0xfffffe0504df09e0
  vpanic() at 0xffffffff8063986c = vpanic+0x14c/frame 0xfffffe0504df0a20
  panic() at 0xffffffff806395b3 = panic+0x43/frame 0xfffffe0504df0a80
  __mtx_assert() at 0xffffffff8061c40d = __mtx_assert+0xed/frame 0xfffffe0504df0ac0
  sbp_cam_scan_lun() at 0xffffffff80474667 = sbp_cam_scan_lun+0x37/frame 0xfffffe0504df0af0
  xpt_done_process() at 0xffffffff802aacfa = xpt_done_process+0x2da/frame 0xfffffe0504df0b30
  xpt_done_td() at 0xffffffff802ac2e5 = xpt_done_td+0xd5/frame 0xfffffe0504df0b80
  fork_exit() at 0xffffffff805ff72f = fork_exit+0xdf/frame 0xfffffe0504df0bf0
  fork_trampoline() at 0xffffffff8082483e = fork_trampoline+0xe/frame
  0xfffffe0504df0bf0
  --- trap 0, rip = 0, rsp = 0, rbp = 0 ---

Also, I tried to reduce the scope of the sbp lock to avoid holding it
while doing bus_dma allocations.

The code badly needs some re-engineering.  SBP really should implement
a CAM transport, so that it avoids control flow inversion when re-scanning
the bus.  Also, the sbp lock seems to be too coarse.

Additionally, the commit includes some changes not related to locking.

- sbp_cam_scan_lun: restore CAM_DEV_QFREEZE before re-queueing the ccb
  because xpt_setup_ccb resets ccb_h.flags
- sbp_post_busreset: call xpt_release_simq only if it's actually frozen
- don't place private SIMQ_FREEZED flag (sic, "freezed") into sim-&gt;flags,
  use sbp-&gt;flags for that
- some style fixes and control flow enhancements

Reviewed by:	sbruno
MFC after:	2 weeks
Differential Revision: https://reviews.freebsd.org/D9898
</pre>
</div>
</content>
</entry>
<entry>
<title>Add safety check against too long CDB.</title>
<updated>2017-02-28T05:17:50+00:00</updated>
<author>
<name>Alexander Motin</name>
<email>mav@FreeBSD.org</email>
</author>
<published>2017-02-28T05:17:50+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=0e672f795efc0afb318ca41777402eee798427e3'/>
<id>0e672f795efc0afb318ca41777402eee798427e3</id>
<content type='text'>
SBP-2 specification defined maximum CDB length as 12 bytes.  Newer SBP-3
specification allows CDB of any size, but this driver is too old.  Proper
solution would be to look on maximal ORB size supported by the target.

MFC after:	1 week
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
SBP-2 specification defined maximum CDB length as 12 bytes.  Newer SBP-3
specification allows CDB of any size, but this driver is too old.  Proper
solution would be to look on maximal ORB size supported by the target.

MFC after:	1 week
</pre>
</div>
</content>
</entry>
<entry>
<title>In the same vein as r311350, fix whitespace in handling of XPT_PATH_INQ in</title>
<updated>2017-01-05T03:08:57+00:00</updated>
<author>
<name>Ravi Pokala</name>
<email>rpokala@FreeBSD.org</email>
</author>
<published>2017-01-05T03:08:57+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=d3c06026c2bac2ca95e210400df2d0002c735d9a'/>
<id>d3c06026c2bac2ca95e210400df2d0002c735d9a</id>
<content type='text'>
several more drivers.

Sponsored by:	Panasas
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
several more drivers.

Sponsored by:	Panasas
</pre>
</div>
</content>
</entry>
<entry>
<title>Always null-terminate ccb_pathinq.(sim_vid|hba_vid|dev_name)</title>
<updated>2017-01-04T20:26:42+00:00</updated>
<author>
<name>Alan Somers</name>
<email>asomers@FreeBSD.org</email>
</author>
<published>2017-01-04T20:26:42+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=4195c7de2471d6212744bcbf128d659e94801d17'/>
<id>4195c7de2471d6212744bcbf128d659e94801d17</id>
<content type='text'>
The sim_vid, hba_vid, and dev_name fields of struct ccb_pathinq are
fixed-length strings. AFAICT the only place they're read is in
sbin/camcontrol/camcontrol.c, which assumes they'll be null-terminated.
However, the kernel doesn't null-terminate them. A bunch of copy-pasted code
uses strncpy to write them, and doesn't guarantee null-termination. For at
least 4 drivers (mpr, mps, ciss, and hyperv), the hba_vid field actually
overflows. You can see the result by doing "camcontrol negotiate da0 -v".

This change null-terminates those fields everywhere they're set in the
kernel. It also shortens a few strings to ensure they'll fit within the
16-character field.

PR:		215474
Reported by:	Coverity
CID:		1009997 1010000 1010001 1010002 1010003 1010004 1010005
CID:		1331519 1010006 1215097 1010007 1288967 1010008 1306000
CID:		1211924 1010009 1010010 1010011 1010012 1010013 1010014
CID:		1147190 1010017 1010016 1010018 1216435 1010020 1010021
CID:		1010022 1009666 1018185 1010023 1010025 1010026 1010027
CID:		1010028 1010029 1010030 1010031 1010033 1018186 1018187
CID:		1010035 1010036 1010042 1010041 1010040 1010039
Reviewed by:	imp, sephe, slm
MFC after:	4 weeks
Sponsored by:	Spectra Logic Corp
Differential Revision:	https://reviews.freebsd.org/D9037
Differential Revision:	https://reviews.freebsd.org/D9038
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The sim_vid, hba_vid, and dev_name fields of struct ccb_pathinq are
fixed-length strings. AFAICT the only place they're read is in
sbin/camcontrol/camcontrol.c, which assumes they'll be null-terminated.
However, the kernel doesn't null-terminate them. A bunch of copy-pasted code
uses strncpy to write them, and doesn't guarantee null-termination. For at
least 4 drivers (mpr, mps, ciss, and hyperv), the hba_vid field actually
overflows. You can see the result by doing "camcontrol negotiate da0 -v".

This change null-terminates those fields everywhere they're set in the
kernel. It also shortens a few strings to ensure they'll fit within the
16-character field.

PR:		215474
Reported by:	Coverity
CID:		1009997 1010000 1010001 1010002 1010003 1010004 1010005
CID:		1331519 1010006 1215097 1010007 1288967 1010008 1306000
CID:		1211924 1010009 1010010 1010011 1010012 1010013 1010014
CID:		1147190 1010017 1010016 1010018 1216435 1010020 1010021
CID:		1010022 1009666 1018185 1010023 1010025 1010026 1010027
CID:		1010028 1010029 1010030 1010031 1010033 1018186 1018187
CID:		1010035 1010036 1010042 1010041 1010040 1010039
Reviewed by:	imp, sephe, slm
MFC after:	4 weeks
Sponsored by:	Spectra Logic Corp
Differential Revision:	https://reviews.freebsd.org/D9037
Differential Revision:	https://reviews.freebsd.org/D9038
</pre>
</div>
</content>
</entry>
<entry>
<title>Prevent overflow issues in timeout processing</title>
<updated>2014-11-21T21:01:24+00:00</updated>
<author>
<name>Steven Hartland</name>
<email>smh@FreeBSD.org</email>
</author>
<published>2014-11-21T21:01:24+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=85c9dd9d895261632d58cf98da6608b93dd5d7f8'/>
<id>85c9dd9d895261632d58cf98da6608b93dd5d7f8</id>
<content type='text'>
Previously, any timeout value for which (timeout * hz) will overflow the
signed integer, will give weird results, since callout(9) routines will
convert negative values of ticks to '1'. For unsigned integer overflow we
will get sufficiently smaller timeout values than expected.

Switch from callout_reset, which requires conversion to int based ticks
to callout_reset_sbt to avoid this.

Also correct isci to correctly resolve ccb timeout.

This was based on the original work done by Eygene Ryabinkin
&lt;rea@freebsd.org&gt; back in 5 Aug 2011 which used a macro to help avoid
the overlow.

Differential Revision:	https://reviews.freebsd.org/D1157
Reviewed by:	mav, davide
MFC after:	1 month
Sponsored by:	Multiplay
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Previously, any timeout value for which (timeout * hz) will overflow the
signed integer, will give weird results, since callout(9) routines will
convert negative values of ticks to '1'. For unsigned integer overflow we
will get sufficiently smaller timeout values than expected.

Switch from callout_reset, which requires conversion to int based ticks
to callout_reset_sbt to avoid this.

Also correct isci to correctly resolve ccb timeout.

This was based on the original work done by Eygene Ryabinkin
&lt;rea@freebsd.org&gt; back in 5 Aug 2011 which used a macro to help avoid
the overlow.

Differential Revision:	https://reviews.freebsd.org/D1157
Reviewed by:	mav, davide
MFC after:	1 month
Sponsored by:	Multiplay
</pre>
</div>
</content>
</entry>
<entry>
<title>Remove obsolete compatibility glue and improve firewire code readability.</title>
<updated>2014-09-27T16:50:21+00:00</updated>
<author>
<name>Alexander Kabaev</name>
<email>kan@FreeBSD.org</email>
</author>
<published>2014-09-27T16:50:21+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=23667f089d0aaf2c46f7d80ef5648cfa60f30cd0'/>
<id>23667f089d0aaf2c46f7d80ef5648cfa60f30cd0</id>
<content type='text'>
Commit my version of style(9) pass over the firewire code. Now that
other people have started changing the code carrying this is as a
local patch is not longer a viable option.

MFC after:	1 month
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Commit my version of style(9) pass over the firewire code. Now that
other people have started changing the code carrying this is as a
local patch is not longer a viable option.

MFC after:	1 month
</pre>
</div>
</content>
</entry>
<entry>
<title>Start the process of cleaning up FreeBSD's firewire driver.</title>
<updated>2014-09-18T17:28:21+00:00</updated>
<author>
<name>Will Andrews</name>
<email>will@FreeBSD.org</email>
</author>
<published>2014-09-18T17:28:21+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=138ebd1b6403435182280641ccacf08fe58297d6'/>
<id>138ebd1b6403435182280641ccacf08fe58297d6</id>
<content type='text'>
sys/dev/firewire/firewire.c:
sys/dev/firewire/firewire.h:
sys/dev/firewire/firewirereg.h:
sys/dev/firewire/fwcrom.c:
sys/dev/firewire/fwdev.c:
sys/dev/firewire/fwdma.c:
sys/dev/firewire/fwmem.c:
sys/dev/firewire/fwohci.c:
sys/dev/firewire/fwohci_pci.c:
sys/dev/firewire/fwohcivar.h:
sys/dev/firewire/if_fwe.c:
sys/dev/firewire/if_fwip.c:
sys/dev/firewire/sbp.c:
sys/dev/firewire/sbp_targ.c:
	Unifdef the code, removing support for DragonflyBSD
	and FreeBSD prior to version 5.

Submitted by:	gibbs
MFC after:	1 month
Sponsored by:	Spectra Logic
MFSpectraBSD:	1081188 on 2014/08/01
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
sys/dev/firewire/firewire.c:
sys/dev/firewire/firewire.h:
sys/dev/firewire/firewirereg.h:
sys/dev/firewire/fwcrom.c:
sys/dev/firewire/fwdev.c:
sys/dev/firewire/fwdma.c:
sys/dev/firewire/fwmem.c:
sys/dev/firewire/fwohci.c:
sys/dev/firewire/fwohci_pci.c:
sys/dev/firewire/fwohcivar.h:
sys/dev/firewire/if_fwe.c:
sys/dev/firewire/if_fwip.c:
sys/dev/firewire/sbp.c:
sys/dev/firewire/sbp_targ.c:
	Unifdef the code, removing support for DragonflyBSD
	and FreeBSD prior to version 5.

Submitted by:	gibbs
MFC after:	1 month
Sponsored by:	Spectra Logic
MFSpectraBSD:	1081188 on 2014/08/01
</pre>
</div>
</content>
</entry>
<entry>
<title>Pull in r267961 and r267973 again. Fix for issues reported will follow.</title>
<updated>2014-06-28T03:56:17+00:00</updated>
<author>
<name>Hans Petter Selasky</name>
<email>hselasky@FreeBSD.org</email>
</author>
<published>2014-06-28T03:56:17+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=af3b2549c4ba2ef00a7cbb4cb6836598bf0aefbe'/>
<id>af3b2549c4ba2ef00a7cbb4cb6836598bf0aefbe</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Revert r267961, r267973:</title>
<updated>2014-06-27T22:05:21+00:00</updated>
<author>
<name>Glen Barber</name>
<email>gjb@FreeBSD.org</email>
</author>
<published>2014-06-27T22:05:21+00:00</published>
<link rel='alternate' type='text/html' href='http://cgit.freebsd.org/src/commit/?id=37a107a407cdb47ee0f4c4337e369e9973b34076'/>
<id>37a107a407cdb47ee0f4c4337e369e9973b34076</id>
<content type='text'>
These changes prevent sysctl(8) from returning proper output,
such as:

 1) no output from sysctl(8)
 2) erroneously returning ENOMEM with tools like truss(1)
    or uname(1)
 truss: can not get etype: Cannot allocate memory
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
These changes prevent sysctl(8) from returning proper output,
such as:

 1) no output from sysctl(8)
 2) erroneously returning ENOMEM with tools like truss(1)
    or uname(1)
 truss: can not get etype: Cannot allocate memory
</pre>
</div>
</content>
</entry>
</feed>
