On Wed, Sep 26, 2018 at 10:34:45AM -0600, Jason Gunthorpe wrote: > 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.. Delete all prints from all drivers? Add lock/unlock for every access to dev_name? Just kidding, it looks like we will need to leave with this situation, it is expected to be very rare one and will cause to issues in debug kernels only, where in theory, it can panic after such access. In productions kernels without checkers, use-after-free is silently ignored. Thanks > > Jason
Attachment:
signature.asc
Description: PGP signature
