RE: [PATCH 07/19] Fix RCU handling of scsi_device.vpd_pg8[03]

Seymour, Shane M
2017-08-28T02:02:32Z

> Hello Shane,
>
> You have either misinterpret my statement or the SCSI VPD handling code. If
> you have a look at the SCSI VPD handling code you will see that an
> rcu_read_lock() /
> rcu_read_unlock() pair is sufficient to prevent that the VPD buffer
> rcu_dereference() points at is being modified as long as the RCU read lock is
> held, at least if
> rcu_dereference() is only called once. The update side namely does not
> modify the VPD buffer after the pointer to that buffer has been published.

Hi Bart,

I'm pretty sure I understood the code. My main point was that the only thing that RCU guarantees with code in between rcu_read_lock()/rcu_read_unlock() if you dereference an RCU pointer using rcu_dereference() is that the memory that the pointer returned points to won't be freed and will be valid regardless of if you get the new or old pointer. Anything related to the actual contents of what the pointer points to is code specific in regard to what happens to it.

As Christoph pointed out (which I failed to consider fully) by doing a direct kfree in scsi_device_dev_release_usercontext() without a call to synchronize_rcu() if you had some code that that got one of the RCU pointers in a read-side critical section and for some reason scsi_device_dev_release_usercontext() got called you could now have an invalid pointer when RCU makes a guarantee that it must be valid. That's what I believe Christoph was discussing in his reply to my email.

> Switching to kfree_rcu() requires more changes because all unsigned char
> pointers to VPD data have to be converted into pointers to a structure that
> contains the VPD data and the RCU head. Anyway, I will convert the kfree()
> calls to RCU pointers into kfree_rcu() pointers.
>

Thanks, I appreciate you taking the comments onboard. I've been looking at the new patches today and should post something back by the end of my day today.

Shane