On Wed, Sep 26, 2018 at 07:25:10PM +0300, Leon Romanovsky wrote: > On Wed, Sep 26, 2018 at 10:02:36AM -0600, Jason Gunthorpe wrote: > > On Wed, Sep 26, 2018 at 06:34:23PM +0300, Leon Romanovsky wrote: > > > > > > So what are the "downsides" to calling this function? I think you should > > > > mention that in the commit message and make the justification for why > > > > this is OK rather than, someone else did so we can to. > > > > > > Actually, I reread again the comment above device_rename() and think > > > that "downsides" mentioned there can be races with symlinks only. > > > > > > We are holding lock which prevent addition of new ib_device with same > > > name, so from name point of view, we are safe. > > > > > > Regarding comment, I don't know what else can I add to comment in > > > device_rename() section. > > > > The bigger downside is that all the places that use dev_name will > > use after free if they race with device_rename(), but that bug > > afflicts netdev to some degree as well, though netdev often (but not > > always) uses netdev_name() that doesn't have this race. > > How? We are holding device_lock which will prevent release of the device. Because device_rename calls kobject_set_name_vargs which does: s = kvasprintf_const(GFP_KERNEL, fmt, vargs); kfree_const(kobj->name); kobj->name = s; And dev_name does: return kobj->name; So, it use after frees if you race the two functions. netdev has this bug, I see no particularly obvious solution.. Jason
