On Mon, 12 Oct 2015, Kirill A. Shutemov wrote:
On Mon, Oct 12, 2015 at 10:49:45AM -0700, Davidlohr Bueso wrote:
diff --git a/ipc/shm.c b/ipc/shm.c
index 4178727..9615f19 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -385,9 +385,25 @@ static struct mempolicy *shm_get_policy(struct
vm_area_struct *vma,
static int shm_mmap(struct file *file, struct vm_area_struct *vma)
{
- struct shm_file_data *sfd = shm_file_data(file);
+ struct file *vma_file = vma->vm_file;
+ struct shm_file_data *sfd = shm_file_data(vma_file);
+ struct ipc_ids *ids = &shm_ids(sfd->ns);
+ struct kern_ipc_perm *shp;
int ret;
+ rcu_read_lock();
+ shp = ipc_obtain_object_check(ids, sfd->id);
+ if (IS_ERR(shp)) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (!ipc_valid_object(shp)) {
+ ret = -EIDRM;
+ goto err;
+ }
+ rcu_read_unlock();
+
Hm. Isn't it racy? What prevents IPC_RMID from happening after this point?
Nothing, but that is later caught by shm_open() doing similar checks. We
basically end up doing a check between ->mmap() calls, which is fair imho.
Note that this can occur anywhere in ipc as IPC_RMID is a user request/cmd,
and we try to respect it -- thus you can argue this race anywhere, which is
why we have EIDRM/EINVL. Ultimately the user should not be doing such hacks
_anyway_. So I'm not really concerned about it.
Another similar alternative would be perhaps to make shm_lock() return an
error, and thus propagate that error to mmap return. That way we would have
a silent way out of the warning scenario (afterward we cannot race as we
hold the ipc object lock). However, the users would now have to take this
into account...
[validity check lockless]
->mmap()
[validity check lock]
Shouldn't we bump shm_nattch here? Or some other refcount?
At least not shm_nattach, as that would acknowledge a new attachment after
a valid IPC_RMID. But the problem is also with how we check for marked for
deletion segments -- ipc_valid_object() checking the deleted flag. As such,
we always rely on explicitly checking against the deleted flag.
--
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/
