netdev - RE: [RDMA RFC v6 14/16] RDMA/irdma: Add ABI definitions

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

Date:   Wed, 27 May 2020 01:58:14 +0000
From:   "Saleem, Shiraz" <shiraz.saleem@...el.com>
To:     Jason Gunthorpe <jgg@...lanox.com>,
        Gal Pressman <galpress@...zon.com>
CC:     Greg KH <gregkh@...uxfoundation.org>,
        "Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "Ismail, Mustafa" <mustafa.ismail@...el.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        "poswald@...e.com" <poswald@...e.com>
Subject: RE: [RDMA RFC v6 14/16] RDMA/irdma: Add ABI definitions

> Subject: Re: [RDMA RFC v6 14/16] RDMA/irdma: Add ABI definitions
> 
> On Wed, May 20, 2020 at 12:02:35PM +0300, Gal Pressman wrote:
> > On 20/05/2020 11:52, Greg KH wrote:
> > > On Wed, May 20, 2020 at 10:54:25AM +0300, Gal Pressman wrote:
> > >> On 20/05/2020 10:04, Jeff Kirsher wrote:
> > >>> +struct i40iw_create_qp_resp {
> > >>> +   __u32 qp_id;
> > >>> +   __u32 actual_sq_size;
> > >>> +   __u32 actual_rq_size;
> > >>> +   __u32 i40iw_drv_opt;
> > >>> +   __u16 push_idx;
> > >>> +   __u8 lsmm;
> > >>> +   __u8 rsvd;
> > >>> +};
> > >>
> > >> This struct size should be 8 bytes aligned.
> > >
> > > Aligned in what way?  Seems sane to me, what would you want it to
> > > look like instead?
> >
> > The uverbs ABI structs sizes are assumed to be padded to 8 bytes
> > alignment, I would expect the reserved field to be an array of 5 bytes
> > as done in other structs in this file (irdma_modify_qp_req for example).
> > Jason could correct me if I'm wrong?
> 
> "it is complicated"
> 
> The udata structs must have alignment that is compatible with the core struct that
> prefixes them. Of course we have a mess here, and nothing is uniform..
> 
> In this case struct ib_uverbs_create_qp_resp has a '__u32 driver_data[0]' aligned
> to 8 bytes thus the alignment of this struct can be 4 or 8.
> 
> I generally don't recommend relying on this weird side effect, and encourage
> explicit padding when possible, but since the intent of this new driver is to be ABI
> compatible with the old driver, it should be kept the same.
> 
> The userspace has a number of static_asserts which are designed to automatically
> check these various cases. I assume Intel has revised the userspace to use the
> new struct names and tested it..
> 

Thanks Jason for the explanation! Yes these abi structs are kept the same for old user-space compatibility. And yes its been tested with old user-space.