tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: VOP_STRATEGY for devices and pipes



Seems to me the best/safest thing to do for now would be to prohibit
the extended-attr on devices and fifos.

On Thu, 15 Jul 2021, David Holland wrote:

I noticed that there was a loose function ffsext_strategy lying
around, mentioned it to Christos, and he committed this:

> Modified Files:
> 	src/sys/ufs/ffs: ffs_vnops.c
>
> Log Message:
> Hook up ffsext_strategy to fifos. Pointed out by dholland@

which is well and good (actually it isn't, it blew up a vnode tables
cleanup patch I was preparing, but never mind that for now) but:

If this is needed for fifos, isn't it also needed for devices?

The ffs extended attribute blocks are stored in the buffer cache as
negative offsets into the file they belong to (distinct, one hopes,
from the negative offsets used for indirect blocks), which means that
sooner or later they'll get written out, and when they do that happens
via VOP_STRATEGY on the vnode they belong to.

For fifos, without the above change, this will panic; VOP_STRATEGY on
all fifos was/is genfs_badop. So probably trying to set extended
attributes on a fifo will crash.

For devices, though, the same thing will happen: buffers at negative
offsets will be hung on the device, but if not intercepted by ffs
(currently they aren't) they'll be handled by spec_strategy and
treated as physical disk blocks. Which if we're lucky, will crash, and
if not, possibly write to negative offsets in one disk partition which
are positive offsets in the previous one, and corrupt the filesystem.

So I think ffs also needs an ffsspec_strategy that does the same
interception.

Except, I'm not convinced that it's a good idea to try to mix the
filesystem's metadata blocks with the device's own raw blocks like
this. I don't know of anything that tries to use negative offsets on a
device to mean something special, but I don't know there isn't
anything either, in which case combining the two would make a horrible
mess. But even absent that, it seems like having these entirely
different blocks queueing in the same place is a bad idea begging for
trouble.

So I think what I would like to do is, for now at least (the device
plumbing changes I posted about a couple months back would eventually
change the situation) just prohibit extended attributes on device
nodes entirely.

(I would not be opposed to also prohibiting them on fifos and
reverting this change for the time being, if only because it means I
wouldn't have to redo the patches I was hoping to commit this week...)

Thoughts? Also, am I missing something?

--
David A. Holland
dholland%netbsd.org@localhost

!DSPAM:60efa3bd146021739713255!



+--------------------+--------------------------+----------------------+
| Paul Goyette       | PGP Key fingerprint:     | E-mail addresses:    |
| (Retired)          | FA29 0E3B 35AF E8AE 6651 | paul%whooppee.com@localhost    |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoyette%netbsd.org@localhost  |
|                    |                          | pgoyette99%gmail.com@localhost |
+--------------------+--------------------------+----------------------+


Home | Main Index | Thread Index | Old Index