Skip to content

Commit 1ac0b6d

Browse files
kiryltorvalds
authored andcommitted
ipc/shm: handle removed segments gracefully in shm_mmap()
remap_file_pages(2) emulation can reach file which represents removed IPC ID as long as a memory segment is mapped. It breaks expectations of IPC subsystem. Test case (rewritten to be more human readable, originally autogenerated by syzkaller[1]): #define _GNU_SOURCE #include <stdlib.h> #include <sys/ipc.h> #include <sys/mman.h> #include <sys/shm.h> #define PAGE_SIZE 4096 int main() { int id; void *p; id = shmget(IPC_PRIVATE, 3 * PAGE_SIZE, 0); p = shmat(id, NULL, 0); shmctl(id, IPC_RMID, NULL); remap_file_pages(p, 3 * PAGE_SIZE, 0, 7, 0); return 0; } The patch changes shm_mmap() and code around shm_lock() to propagate locking error back to caller of shm_mmap(). [1] http://github.com/google/syzkaller Signed-off-by: Kirill A. Shutemov <[email protected]> Reported-by: Dmitry Vyukov <[email protected]> Cc: Davidlohr Bueso <[email protected]> Cc: Manfred Spraul <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 64f0085 commit 1ac0b6d

File tree

1 file changed

+43
-10
lines changed

1 file changed

+43
-10
lines changed

ipc/shm.c

+43-10
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,12 @@ static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id)
156156
struct kern_ipc_perm *ipcp = ipc_lock(&shm_ids(ns), id);
157157

158158
/*
159-
* We raced in the idr lookup or with shm_destroy(). Either way, the
160-
* ID is busted.
159+
* Callers of shm_lock() must validate the status of the returned ipc
160+
* object pointer (as returned by ipc_lock()), and error out as
161+
* appropriate.
161162
*/
162-
WARN_ON(IS_ERR(ipcp));
163-
163+
if (IS_ERR(ipcp))
164+
return (void *)ipcp;
164165
return container_of(ipcp, struct shmid_kernel, shm_perm);
165166
}
166167

@@ -186,18 +187,33 @@ static inline void shm_rmid(struct ipc_namespace *ns, struct shmid_kernel *s)
186187
}
187188

188189

189-
/* This is called by fork, once for every shm attach. */
190-
static void shm_open(struct vm_area_struct *vma)
190+
static int __shm_open(struct vm_area_struct *vma)
191191
{
192192
struct file *file = vma->vm_file;
193193
struct shm_file_data *sfd = shm_file_data(file);
194194
struct shmid_kernel *shp;
195195

196196
shp = shm_lock(sfd->ns, sfd->id);
197+
198+
if (IS_ERR(shp))
199+
return PTR_ERR(shp);
200+
197201
shp->shm_atim = get_seconds();
198202
shp->shm_lprid = task_tgid_vnr(current);
199203
shp->shm_nattch++;
200204
shm_unlock(shp);
205+
return 0;
206+
}
207+
208+
/* This is called by fork, once for every shm attach. */
209+
static void shm_open(struct vm_area_struct *vma)
210+
{
211+
int err = __shm_open(vma);
212+
/*
213+
* We raced in the idr lookup or with shm_destroy().
214+
* Either way, the ID is busted.
215+
*/
216+
WARN_ON_ONCE(err);
201217
}
202218

203219
/*
@@ -260,13 +276,22 @@ static void shm_close(struct vm_area_struct *vma)
260276
down_write(&shm_ids(ns).rwsem);
261277
/* remove from the list of attaches of the shm segment */
262278
shp = shm_lock(ns, sfd->id);
279+
280+
/*
281+
* We raced in the idr lookup or with shm_destroy().
282+
* Either way, the ID is busted.
283+
*/
284+
if (WARN_ON_ONCE(IS_ERR(shp)))
285+
goto done; /* no-op */
286+
263287
shp->shm_lprid = task_tgid_vnr(current);
264288
shp->shm_dtim = get_seconds();
265289
shp->shm_nattch--;
266290
if (shm_may_destroy(ns, shp))
267291
shm_destroy(ns, shp);
268292
else
269293
shm_unlock(shp);
294+
done:
270295
up_write(&shm_ids(ns).rwsem);
271296
}
272297

@@ -388,17 +413,25 @@ static int shm_mmap(struct file *file, struct vm_area_struct *vma)
388413
struct shm_file_data *sfd = shm_file_data(file);
389414
int ret;
390415

416+
/*
417+
* In case of remap_file_pages() emulation, the file can represent
418+
* removed IPC ID: propogate shm_lock() error to caller.
419+
*/
420+
ret =__shm_open(vma);
421+
if (ret)
422+
return ret;
423+
391424
ret = sfd->file->f_op->mmap(sfd->file, vma);
392-
if (ret != 0)
425+
if (ret) {
426+
shm_close(vma);
393427
return ret;
428+
}
394429
sfd->vm_ops = vma->vm_ops;
395430
#ifdef CONFIG_MMU
396431
WARN_ON(!sfd->vm_ops->fault);
397432
#endif
398433
vma->vm_ops = &shm_vm_ops;
399-
shm_open(vma);
400-
401-
return ret;
434+
return 0;
402435
}
403436

404437
static int shm_release(struct inode *ino, struct file *file)

0 commit comments

Comments
 (0)