Potential Data Race: a lock should be acquired before calling bhv_base_unlocked according to comments before bhv_base. However, in the following call chain, no lock is acquired before calling 435 bdp = bhv_base_unlocked(VN_BHV_HEAD(vp)); (This code is in file fs/xfs/linux-2.6/xfs_ioctl.c) The call chain is: xfs_ioctl calls xfs_fssetdm_by_handle calls bhv_base_unlocked In the entire call chain, no lock is acquired for calling bhv_base_unlocked, and xfs_ioctl can be exposed functions outside the module, which can be very dangerous.
behaviors don't have locks for local mode XFS since the chain is only one level deep and never changed. The unlock part of the name is misleading. All that that call is doing is returning the last behavior in the chain. xfs_behavior.h:185 /* No bhv locking on Linux */ #define bhv_base_unlocked bhv_base
Check the comments before bhv_base() (attached). It suggests acquiring the lock. 157 /* 158 * Return the base behavior in the chain, or NULL if the chain 159 * is empty. 160 * 161 * The caller has not read locked the behavior chain, so acquire the 162 * lock before traversing the chain. 163 */ 164 bhv_desc_t * 165 bhv_base(bhv_head_t *bhp) 166 { 167 bhv_desc_t *curdesc; 168 169 for (curdesc = bhp->bh_first; 170 curdesc != NULL; 171 curdesc = curdesc->bd_next) { 172 173 if (curdesc->bd_next == NULL) { 174 return curdesc; 175 } 176 } 177 178 return NULL; 179 }
That comment is a left over from the original version of bhv_base_unlocked. The function originally would lock the behavior chain when doing the lookup here, the the unlocked portion of the name. The unlocked in the function name was to signify the caller has not taken a read lock on the behavior chain and therefore the function is required to take the lock. One the linux the behavior chains are static once they are set up at mount time and not dynamically changing, therefore the locks were dropped from the linux implementation of behaviors. That comment should probably be removed as it is mis-leading.
I agree. Thanks.
Could someone please update the wrong comment? Thanks.
This whole mess is gone now. One way to close a bug, I guess :)