linux-kernel - Re: [PATCH net-next v2 1/4] vm_sockets: Include flags field in the vsock address data structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <265d2382-3909-cf1d-f3af-4fd14278ad4c@amazon.com>
Date:   Thu, 10 Dec 2020 17:29:27 +0200
From:   "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Stefano Garzarella <sgarzare@...hat.com>
CC:     netdev <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "David S . Miller" <davem@...emloft.net>,
        David Duncan <davdunc@...zon.com>,
        Dexuan Cui <decui@...rosoft.com>,
        Alexander Graf <graf@...zon.de>,
        Jorgen Hansen <jhansen@...are.com>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>
Subject: Re: [PATCH net-next v2 1/4] vm_sockets: Include flags field in the
 vsock address data structure



On 09/12/2020 19:30, Jakub Kicinski wrote:
> On Wed, 9 Dec 2020 17:17:56 +0200 Paraschiv, Andra-Irina wrote:
>>> I agree that could be a problem, but here some considerations:
>>> - I checked some applications (qemu-guest-agent, ncat, iperf-vsock) and
>>>    all use the same pattern: allocate memory, initialize all the
>>>    sockaddr_vm to zero (to be sure to initialize the svm_zero), set the
>>>    cid and port fields.
>>>    So we should be safe, but of course it may not always be true.
>>>
>>> - For now the issue could affect only nested VMs. We introduced this
>>>    support one year ago, so it's something new and maybe we don't cause
>>>    too many problems.
>>>
>>> As an alternative, what about using 1 or 2 bytes from svm_zero[]?
>>> These must be set at zero, even if we only check the first byte in the
>>> kernel.
>> Thanks for the follow-up info.
>>
>> We can also consider the "svm_zero" option and could use 2 bytes from
>> that field for "svm_flags", keeping the same "unsigned short" type.
> Or use svm_zero as a gate for interpreting other fields?
> If svm_zero[0]* == something start checking the value of reserved1?
> * in practice the name can be unioned to something more palatable ;)

Thanks for the shared option, that could be one case to reuse the 
reserved field, with a two phase check logic.

I'll give it a try to the option of having a new field "svm_flags" and 
the "svm_zero" updated and then send out a new revision. Just let me 
know if there are other updates needed / questions in the meantime.


struct sockaddr_vm {
     __kernel_sa_family_t svm_family;
     unsigned short svm_reserved1;
     unsigned int svm_port;
     unsigned int svm_cid;
     unsigned short svm_flags;
     unsigned char svm_zero[sizeof(struct sockaddr) -
                    sizeof(sa_family_t) -
                    sizeof(unsigned short) -
                    sizeof(unsigned int) - sizeof(unsigned int) -
sizeof(unsigned short)];
};


Thanks,
Andra



Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.