> -----Original Message----- > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma- > owner@xxxxxxxxxxxxxxx] On Behalf Of Ruhl, Michael J > Sent: Monday, March 12, 2018 2:59 PM > To: Leon Romanovsky <leon@xxxxxxxxxx>; Doug Ledford > <dledford@xxxxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxxxxxx> > Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux- > rdma@xxxxxxxxxxxxxxx>; Aviad Yehezkel <aviadye@xxxxxxxxxxxx>; Boris > Pismenny <borisp@xxxxxxxxxxxx>; Matan Barak <matanb@xxxxxxxxxxxx>; > Yishai Hadas <yishaih@xxxxxxxxxxxx> > Subject: RE: [PATCH rdma-next 05/12] IB/uverbs: Safely extend existing > attributes > > Hi Matan, > > Some comments in line. > > > -----Original Message----- > > From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Leon Romanovsky > > Sent: Thursday, March 8, 2018 12:19 PM > > To: Doug Ledford <dledford@xxxxxxxxxx>; Jason Gunthorpe > > <jgg@xxxxxxxxxxxx> > > Cc: Leon Romanovsky <leonro@xxxxxxxxxxxx>; RDMA mailing list <linux- > > rdma@xxxxxxxxxxxxxxx>; Aviad Yehezkel <aviadye@xxxxxxxxxxxx>; Boris > > Pismenny <borisp@xxxxxxxxxxxx>; Matan Barak > <matanb@xxxxxxxxxxxx>; > > Yishai Hadas <yishaih@xxxxxxxxxxxx> > > Subject: [PATCH rdma-next 05/12] IB/uverbs: Safely extend existing > > attributes > > > > From: Matan Barak <matanb@xxxxxxxxxxxx> > > > > Previously, we've used UVERBS_ATTR_SPEC_F_MIN_SZ for extending > > existing > > attributes. The behavior of this flag was the kernel accepts anything > > bigger than the minimum size it specified. This is unsafe, since in > > order to safely extend an attribute, we need to make sure unknown size > > is zeroed. Replacing UVERBS_ATTR_SPEC_F_MIN_SZ with > > UVERBS_ATTR_SPEC_F_MIN_SZ_OR_ZERO, which essentially checks that > the > > unknown size is zero. In addition, attributes are now decorated with > > UVERBS_ATTR_TYPE and UVERBS_ATTR_STRUCT, so we can provide the > > minimum > > and known length. > > I am not understanding why this check is part of the infrastructure. > > Why is this unsafe? Is there a concern that the incoming data will be passed > back to the user? Ok, I think I see that I misunderstood what this is doing. I understand the comments on safety. You are verifying that there is data available in the PTR_IN before you pass it on to the underlying handler. I still think that this flag should be split into two pieces, so I can avoid the ib_is_buffer_cleared() if I don't need it. Mike > If so, wouldn't it be more simple to just memset() the out going buffer > structure before copying it to the user space? > > This would seem to have a smaller impact than creating a buffer using > memdup_user(), zeroing it, comparing it and then throwing the buffer > away (ib_is_udata_cleared()/ib_is_buffer_cleared()). > > It seems to me that the ib_is_buffer_cleared() function is a bit of a > performance issue. If I have a performance path, is there a way to > do the MIN_SIZE check without having to make sure the PTR_IN is > zeroed? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
