On Thu, Dec 20, 2018 at 03:10:14PM -0600, Steve Wise wrote: > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > Sent: Thursday, December 20, 2018 2:50 PM > > To: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx> > > Cc: dledford@xxxxxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; > > chien.tin.tung@xxxxxxxxx; shiraz.saleem@xxxxxxxxx > > Subject: Re: [PATCH rdma-rc] RDMA/iwcm: Don't copy past the end of > > dev_name() string > > > > On Thu, Dec 20, 2018 at 11:37:54AM -0800, Steve Wise wrote: > > > We now use dev_name(&ib_device->dev) instead of ib_device->name in > > > iwpm messages. The name field in struct device is a const char *, > > > where as ib_device->name is a char array of size IB_DEVICE_NAME_MAX, > > > and it is pre-initialized to zeros. > > > > > > Since iw_cm_map() was using memcpy() to copy in the device name, and > > > copying IWPM_DEVNAME_SIZE bytes, it ends up copying past the device > > > name string and getting random bytes. This results in iwpmd failing > > > the REGISTER_PID request from iwcm. Thus port mapping is broken. > > > > > > The fix is to initialize the iwpm_dev_data message memory to zeros, > > > and then strcopy() to avoid copying past the end of the dev_name() > string. > > > > The fix is to return failure if the name is too long to be held > > in the iwpm_dev_data... > > That would not fix the original memcpy() copying past the end of the struct > device name. It wasn't overflowing the iwpm_dev_data, it was copying past > the end of the device string and copying garbage. Well, you'd use a sensible strncpy and fail if the source string is too long. Handling strings with memcpy is always wrong. Sending a truncated string to userspace is garbage.. > However, if this is to make 4.20-rc, then I recommend we do my proposed > change. Is it too late for 4.20? Yes Jason
