netdev - Re: [RFC PATCH v1 0/1] net: mctp: MCTP VDM extension

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>] [day] [month] [year] [list]

Date:   Fri, 28 Apr 2023 17:45:45 +0800
From:   Jeremy Kerr <jk@...econstruct.com.au>
To:     "Richert, Krzysztof" <krzysztof.richert@...el.com>,
        matt@...econstruct.com.au
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org
Subject: Re: [RFC PATCH v1 0/1] net: mctp: MCTP VDM extension

Hi Krzysztof,

> > However, this may be tricky for recvfrom(), where there is no prior
> > length value available. We could extract this from the earlier
> > (outgoing) message, but that's getting a bit hacky at that point.
> Actually, I've thought to use the same approach as for sockaddr_mctp_ext.
> So subtype data would be part of sockaddr(a), but enabled by MCTP_OPT_VENDOR_EXT.

Yes - you'll always need the setsockopt to enable the different
sockaddr format, but we may need it to also provide the format (length,
whatever) of the subtype addressing. However, there are some
more factors there, I'll cover those below.

> In scenario when bind() function is not called, it would result in receiving
> message from any subtype, right ? 

Yes - if we don't have a sockaddr_mctp_vendor_ext at the time the packet
comes in, then we have no idea how to extract the packet subtype.

> > Are there cases where the subtype does not immediately follow the
> > PCIe/IANA type? If not, we could just require that...
> Yes. Just after PCIe/IANA, in the Intel format, is 'control byte' and
> next subtype. For some vendors it may be, as you assumed, immediately
> follow the PCIe/IANA, or on completely different offset.

OK, the "completely different offset" is a fairly major issue here.

To implement that, we essentially need a sort of arbitrary packet
inspection to allow the addressing mechanism to parse a subtype from any
data in the packet. I can see a few potential issues with that, mainly:

 1) there's no way to prevent a packet matching multiple binds(); there
    may be incoming packets that match more than one subtype if they're
    at different offsets

 2) similarly: there's no way to reject a bind() if it conflicts with
    an existing bound socket

 3) this sounds like something that may be prone to bugs

One alternative I can see is that:

 A) we keep the DSP0236 specified behaviour in the kernel; allowing a
    bind on a PCI/IANA vendor type, but not the subtype

 B) we add a "vendor mark" field to the sockaddr_mctp_vendor_ext; an
    arbitrary u32. Sockets can specify a vendor mark value during bind()
    so that they receive packets with a specific mark. This allows us
    to reject duplicate bind()s on the same mark value.

 C) packets can get a vendor mark applied by a some user-specified BPF
    (or similar?) logic, that can do the arbitrary packet parsing, and
    apply a suitable mark value to the skb.

 D) when looking up the socket for an incoming packet, we also require
    a match on the vendor mark, if one is applied during bind().

With that, we wold just need the appropriate plumbing to allow marking
through BPF. It would be up to userspace to provide the actual parse ->
mark logic, and to match the bind() mark value to whatever the BPF sets.

Would that work?

Cheers,


Jeremy