Re: [PATCH] scsi: eata: drop VLA in reorder()
On Mon, 12 Mar 2018 14:08:34 +1100, "Tobin C. Harding" said: > removal patch that 768 was a lot of stack space. That comment did, > however say 'deep in some transfer call chain'. I don't know what a > 'transfer call chain' (the transfer bit) is but is there some heuristic > we can use to know how deep is deep? Or more to the point, is there some > heuristic we can use to know what is an acceptable amount of stack space > to use? The canonical "why we put kernel stacks on a diet" configuration: Imagine a bunch of ISCSI targets - with IPSec wrapping the connection. Arranged into a software RAID5. With LVM. With encryption on the LV. With XFS on the encrypted LV. And then the in-kernel sharing it out over NFS. With more IPSec wrapping the NFS TCP connection. Oh, and I/O interrupts, just for fun. And most of all of that has to fit their *entire* stack chain into 2 4K pages. Suddenly, that 768 bytes is taking close to 10% of *all* of the stack that all of that call chain has to share. And I see that patch is against scsi/eata.c - which means it can plausibly end up sharing that stack scenario above starting at 'software raid5'. (For bonus points, the alert reader is invited to figure out which stack each of the above actually ends up on. No, it isn't split across enough stacks that taking 768 bytes out of any of them is acceptable.. :) pgpjGzDAruK_Z.pgp Description: PGP signature
UBSAN whinge in scsi_devinfo.c
Seen at boot in a UBSAN-enabled kernel:
[2.936388]
[2.936392] UBSAN: Undefined behaviour in drivers/scsi/scsi_devinfo.c:457:21
[2.936396] index 8 is out of range for type 'char [8]'
The code:
452 if (devinfo->compatible) {
453 /*
454 * Behave like the older version of
get_device_flags.
455 */
456 if (memcmp(devinfo->vendor, vskip, vmax) ||
457 devinfo->vendor[vmax])
458 continue;
459 if (memcmp(devinfo->model, mskip, mmax) ||
460 devinfo->model[mmax])
461 continue;
462 return devinfo;
As near as I can tell, intentionally dereferencing past the end of the
vendor or model strings is well into "just happens to work" - and I'm
convinced this is actually buggy for entries that have 16-character
model identifiers, as the next field is an 'unsigned flags'. And that's
going to fail miserably on a big-endian machine where the flags aren't
in the next byte that follows the 16 chars of model
The entire splat:
[2.936388]
[2.936392] UBSAN: Undefined behaviour in drivers/scsi/scsi_devinfo.c:457:21
[2.936396] index 8 is out of range for type 'char [8]'
[2.936401] CPU: 0 PID: 98 Comm: kworker/u8:1 Not tainted
4.6.0-next-20160517-1-gede618fce89c-dirty #279
[2.936412] Hardware name: Dell Inc. Latitude E6530/07Y85M, BIOS A17
08/19/2015
[2.936424] Workqueue: events_unbound async_run_entry_fn
[2.936432] ce26f4e7 8802236eba38
aa690aea
[2.936439] 8802236eba50 ce26f4e7 8802236eba60
0008
[2.936446] 8802236eba50 aa7087ad abccafc0
8802236ebaa8
[2.936449] Call Trace:
[2.936459] [] dump_stack+0x7b/0xd1
[2.936464] [] ubsan_epilogue+0xd/0x40
[2.936468] [] __ubsan_handle_out_of_bounds+0x75/0xa0
[2.936472] [] ? percpu_down_read_trylock+0xa8/0xb0
[2.936478] [] scsi_dev_info_list_find+0x282/0x300
[2.936482] [] scsi_get_device_flags_keyed+0x21/0xb0
[2.936487] [] scsi_get_device_flags+0x10/0x20
[2.936492] [] scsi_probe_and_add_lun+0x502/0x1200
[2.936497] [] ? _raw_spin_unlock_irqrestore+0x87/0x90
[2.936503] [] __scsi_add_device+0x121/0x150
[2.936510] [] ata_scsi_scan_host+0x127/0x240
[2.936514] [] async_port_probe+0x4a/0x90
[2.936518] [] async_run_entry_fn+0x68/0x1b0
[2.936523] [] process_one_work+0x3bf/0xdb0
[2.936526] [] ? process_one_work+0x329/0xdb0
[2.936531] [] worker_thread+0x351/0xad0
[2.936536] [] ? process_one_work+0xdb0/0xdb0
[2.936540] [] kthread+0x142/0x1b0
[2.936549] [] ret_from_fork+0x1f/0x40
[2.936553] [] ? kthread_create_on_node+0x280/0x280
[2.936557]
pgpTlXV41g3HX.pgp
Description: PGP signature
Re: [PATCH] mvsas:Fix possible NULL pointer deference in mvs_dev_found_notify
On Wed, 09 Mar 2016 22:33:47 +0200, DÄvis MosÄns said: > About whether mvs_find_dev_mvi can return NULL it looks like it's possible, > but I'm not sure if it practically happens. I guess it did hence patch. Or the "bug" was found by incorrect code inspection. Nick has a history of submitting patches that are for either non-existent problems or don't deal with with the issue correctly - bad enough that he's not allowed to post to the linux-kernel list. pgpvvUrIKYeLq.pgp Description: PGP signature
Re: What still uses the block layer?
On Mon, 15 Oct 2007 03:04:00 CDT, Rob Landley said: > I note that the eth0 and eth1 names are dynamically assigned on a first come > first serve basis (like scsi). This never causes me a problem because the > driver loading order is constant, and once you figure out that eth0 is > gigabit and eth1 is the 80211g it _stays_ that way across reboots, reliably. > Yeah, it's a heuristic. Hands up everybody relying on such a heuristic in > the real world. I've gotten burned by that heuristic enough times to not rely on it. My last laptop had an ethernet on the motherboard, a *separate* ethernet in the docking station, an ethernet on a multifunction pcmcia card (I usually just used the modem side), and a wireless that looked like an ethernet - so it was possible for a given interface to be eth1 (if no dock and no pcmcia card) or eth3 (if both were present). And that's on a laptop from almost 5 years ago. And then there's the recent Sun and Dell 1U rack-mounts that have 4 ethernets on the motherboard, and they *never* seem to assign in a 0,1,2,3 order that matches the 0 1 2 3 printed above the 4 RJ45's ;) So I have for years been a proponent of 'ethN is nailed by MAC address' :) pgp7ivUsgAZZU.pgp Description: PGP signature
Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers
On Fri, 31 Aug 2007 16:13:59 BST, Denys Vlasenko said: > >textdata bss dec hex filename > 261433 500181172 312623 4c52f > linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o > 199654 500181172 250844 3d3dc > linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o > 184014 213141172 206500 326a4 > linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o > 20237828501172 206400 32640 > linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o > > 1-debloat.patchdeinlines a lot of functions > 2-addstatic.patch adds statics, #ifdefs out huge amount of unused code, adds > consts > 3-addconst.patch adds more consts Yowza. Looking at aic1->aic2, it looks like 20K became 'const', and only 3K *wasn't* 'const'? pgpAnWeUyBl9V.pgp Description: PGP signature
