On Tue, 2013-12-17 at 19:46 -0200, Rafael Aquini wrote: > On Tue, Dec 17, 2013 at 01:27:49PM -0800, Davidlohr Bueso wrote: > > Ccing Manfred. > > > > On Tue, 2013-12-17 at 17:03 -0200, Rafael Aquini wrote: > > > After the locking semantics for the SysV IPC API got improved, a couple of > > > IPC_RMID race windows were opened because we ended up dropping the > > > 'kern_ipc_perm.deleted' check performed way down in ipc_lock(). > > > The spotted races got sorted out by re-introducing the old test within > > > the racy critical sections. > > > > > > This patch introduces ipc_valid_object() to consolidate the way we cope > > > with > > > IPC_RMID races by using the same abstraction across the API > > > implementation. > > > > This is certainly a good function to have. Some comments below. > > [...] > > > > > > shm_file = shp->shm_file; > > > - > > > - /* check if shm_destroy() is tearing down shp */ > > > - if (shm_file == NULL) { > > > - err = -EIDRM; > > > - goto out_unlock0; > > > - } > > > > Ok, this seems safe, we can always rely on .deleted for validity since > > shm_destroy() ends up calling shm_rmid() which sets .deleted. So this > > change is really moving what we're checking against just a few > > instructions later. > > > > Yep, I did change it cause it seems that there's no reason to delay the return > condition if we raced with shm_destroy(), anyways. >
Right, but I was referring to moving what we consider as valid. static void shm_destroy(struct ipc_namespace *ns, struct shmid_kernel *shp) { struct file *shm_file; shm_file = shp->shm_file; shp->shm_file = NULL; <--- we currently use this. ns->shm_tot -= (shp->shm_segsz + PAGE_SIZE - 1) >> PAGE_SHIFT; shm_rmid(ns, shp); <--- with your patch we now use this. shm_unlock(shp); ... } ... and it makes since since shm was the only one not using .deleted for RMID racing checks. Thanks, Davidlohr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/