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).


macvtap has its own major number, but tuntap use "misc" (10) major number.


Yes.

Another possible solution is to validate the result of TUNGETIFF, but it requires libvirt to tell us the ifname.

Thanks




Thanks,
Laurent




Reply via email to