On 2020/6/30 下午6:35, Laurent Vivier wrote:
On 30/06/2020 12:03, Jason Wang wrote:
On 2020/6/30 下午5:45, Laurent Vivier wrote:
On 30/06/2020 11:31, Daniel P. Berrangé wrote:
On Tue, Jun 30, 2020 at 10:23:18AM +0100, Daniel P. Berrangé wrote:
On Tue, Jun 30, 2020 at 05:21:49PM +0800, Jason Wang wrote:
On 2020/6/30 上午3:30, Laurent Vivier wrote:
On 28/06/2020 08:31, Jason Wang wrote:
On 2020/6/25 下午7:56, Laurent Vivier wrote:
On 25/06/2020 10:48, Daniel P. Berrangé wrote:
On Wed, Jun 24, 2020 at 09:00:09PM +0200, Laurent Vivier wrote:
qemu_set_nonblock() checks that the file descriptor can be
used and, if
not, crashes QEMU. An assert() is used for that. The use of
assert() is
used to detect programming error and the coredump will allow
to debug
the problem.

But in the case of the tap device, this assert() can be
triggered by
a misconfiguration by the user. At startup, it's not a real
problem,
but it
can also happen during the hot-plug of a new device, and here
it's a
problem because we can crash a perfectly healthy system.
If the user/mgmt app is not correctly passing FDs, then there's
a whole
pile of bad stuff that can happen. Checking whether the FD is
valid is
only going to catch a small subset. eg consider if fd=9 refers
to the
FD that is associated with the root disk QEMU has open. We'll
fail to
setup the TAP device and close this FD, breaking the healthy
system
again.

I'm not saying we can't check if the FD is valid, but lets be
clear that
this is not offering very much protection against a broken mgmt
apps
passing bad FDs.

I agree with you, but my only goal here is to avoid the crash in
this
particular case.

The punishment should fit the crime.

The user can think the netdev_del doesn't close the fd, and he
can try
to reuse it. Sending back an error is better than crashing his
system.
After that, if the system crashes, it will be for the good
reasons, not
because of an assert.
Yes. And on top of this we may try to validate the TAP via st_dev
through fstat[1].
I agree, but the problem I have is to know which major(st_dev) we can
allow to use.

Do we allow only macvtap major number?
Macvtap and tuntap.


How to know the macvtap major number at user level?
[it is allocated dynamically: do we need to parse /proc/devices?]
I think we can get them through fstat for /dev/net/tun and
/dev/macvtapX.
Don't assume QEMU has any permission to access to these device nodes,
only the pre-opened FDs it is given by libvirt.
Actually permissions are the least of the problem - the device nodes
won't even exist, because QEMU's almost certainly running in a private
mount namespace with a minimal /dev populated

I'm working on a solution using /proc/devices.

Similar issue with /dev. There's no guarantee that qemu can access
/proc/devices or it may not exist (CONFIG_PROCFS).
There is a lot of things that will not work without /proc (several tools
rely on /proc, like ps, top, lsof, mount, ...). Some information are
only available from /proc, and if /proc is there, I think /proc/devices
is always readable by everyone. Moreover /proc is already used by qemu
in several places.

It can also a best effort check.


Right.



The problem with fstat() on /dev files is to guess the /dev/macvtapX as
X varies (the same with /dev/tapY)..

macvtap has its own major number, but tuntap use "misc" (10) major
number.
Another question: it is possible to use the "fd=" parameter with macvtap
as macvtap creates a /dev/tapY device,


Yes.


but how to do that with tuntap
that does not create a /dev/tapY device?


I think there's no specific reason, it's just because it was wrote like that since the first version which is about 20 years ago.

Thanks



Thanks,
Laurent




Reply via email to