Comment 15 for bug 638955

On Mon, Sep 20, 2010 at 9:31 PM, Anthony Liguori <email address hidden> wrote:
> On 09/20/2010 05:42 AM, Michael S. Tsirkin wrote:
>>
>> On Sun, Sep 19, 2010 at 07:36:51AM +0100, Stefan Hajnoczi wrote:
>>
>>>
>>> On Sat, Sep 18, 2010 at 10:27 PM, Edgar E. Iglesias
>>> <email address hidden>  wrote:
>>>
>>>>
>>>> This doesn't look right. AFAIK, MAC's dont pad on receive.
>>>>
>>>
>>> I agree.  NICs that do padding will do it on transmit, not receive.
>>> Anything coming in on the wire should already have the minimum length.
>>>
>>
>> QEMU never gets access to the wire.
>> Our APIs do not really pass complete ethernet packets:
>> we forward packets without checksum and padding.
>>
>> I think it makes complete sense to keep this and
>> handle padding in devices because we
>> have devices that pass the frame to guest without padding and checksum.
>> It should be easy to replace padding code in devices that
>> need it with some kind of macro.
>>
>
> Would this not also address the problem?  It sounds like the root cause is
> the tap code, not the devices..

This won't work when s->has_vnet_hdr is 1 because the virtio-net
header consumes buffer space and reduces the amount we pad. The
padding size should be 60 + (s->has_vnet_hdr ? sizeof(struct
virtio_net_hdr) : 0).

Adjusting the length without clearing the untouched buffer space is
probably fine. I'm trying to think of a scenario where this becomes
an information leak (security issue). Perhaps if the guest has vlans
enabled and allows different users to sniff traffic only on their
vlans? Then you may be able to read part of another vlan's traffic by
sending short packets to your vlan and gathering the padding data.
This is pretty contrived but doing a <60 byte memset would prevent the
issue for sure.

Stefan