Skip to content

Commit

Permalink
IPC: fix error check in all new xxx_lock() and xxx_exit_ns() functions
Browse files Browse the repository at this point in the history
In the new implementation of the [sem|shm|msg]_lock[_check]() routines, we
use the return value of ipc_lock() in container_of() without any check.
But ipc_lock may return a errcode.  The use of this errcode in
container_of() may alter this errcode, and we don't want this.

And in xxx_exit_ns, the pointer return by idr_find is of type 'struct
kern_ipc_per'...

Today, the code will work as is because the member used in these
container_of() is the first member of its container (offset == 0), the
errcode isn't changed then.  But in the general case, we can't count on
this assumption and this may lead later to a real bug if we don't correct
this.

Again, the proposed solution is simple and correct.  But, as pointed by
Nadia, with this solution, the same check will be done several times (in
all sub-callers...), what is not very funny/optimal...

Signed-off-by: Pierre Peiffer <[email protected]>
Cc: Nadia Derbey <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Pierre Peiffer authored and Linus Torvalds committed Feb 6, 2008
1 parent 15aafa2 commit b1ed88b
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 9 deletions.
17 changes: 14 additions & 3 deletions ipc/msg.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ int msg_init_ns(struct ipc_namespace *ns)
void msg_exit_ns(struct ipc_namespace *ns)
{
struct msg_queue *msq;
struct kern_ipc_perm *perm;
int next_id;
int total, in_use;

Expand All @@ -113,10 +114,11 @@ void msg_exit_ns(struct ipc_namespace *ns)
in_use = msg_ids(ns).in_use;

for (total = 0, next_id = 0; total < in_use; next_id++) {
msq = idr_find(&msg_ids(ns).ipcs_idr, next_id);
if (msq == NULL)
perm = idr_find(&msg_ids(ns).ipcs_idr, next_id);
if (perm == NULL)
continue;
ipc_lock_by_ptr(&msq->q_perm);
ipc_lock_by_ptr(perm);
msq = container_of(perm, struct msg_queue, q_perm);
freeque(ns, msq);
total++;
}
Expand Down Expand Up @@ -144,6 +146,9 @@ static inline struct msg_queue *msg_lock_check_down(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_check_down(&msg_ids(ns), id);

if (IS_ERR(ipcp))
return (struct msg_queue *)ipcp;

return container_of(ipcp, struct msg_queue, q_perm);
}

Expand All @@ -155,6 +160,9 @@ static inline struct msg_queue *msg_lock(struct ipc_namespace *ns, int id)
{
struct kern_ipc_perm *ipcp = ipc_lock(&msg_ids(ns), id);

if (IS_ERR(ipcp))
return (struct msg_queue *)ipcp;

return container_of(ipcp, struct msg_queue, q_perm);
}

Expand All @@ -163,6 +171,9 @@ static inline struct msg_queue *msg_lock_check(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_check(&msg_ids(ns), id);

if (IS_ERR(ipcp))
return (struct msg_queue *)ipcp;

return container_of(ipcp, struct msg_queue, q_perm);
}

Expand Down
17 changes: 14 additions & 3 deletions ipc/sem.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ int sem_init_ns(struct ipc_namespace *ns)
void sem_exit_ns(struct ipc_namespace *ns)
{
struct sem_array *sma;
struct kern_ipc_perm *perm;
int next_id;
int total, in_use;

Expand All @@ -151,10 +152,11 @@ void sem_exit_ns(struct ipc_namespace *ns)
in_use = sem_ids(ns).in_use;

for (total = 0, next_id = 0; total < in_use; next_id++) {
sma = idr_find(&sem_ids(ns).ipcs_idr, next_id);
if (sma == NULL)
perm = idr_find(&sem_ids(ns).ipcs_idr, next_id);
if (perm == NULL)
continue;
ipc_lock_by_ptr(&sma->sem_perm);
ipc_lock_by_ptr(perm);
sma = container_of(perm, struct sem_array, sem_perm);
freeary(ns, sma);
total++;
}
Expand All @@ -181,6 +183,9 @@ static inline struct sem_array *sem_lock_check_down(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_check_down(&sem_ids(ns), id);

if (IS_ERR(ipcp))
return (struct sem_array *)ipcp;

return container_of(ipcp, struct sem_array, sem_perm);
}

Expand All @@ -192,6 +197,9 @@ static inline struct sem_array *sem_lock(struct ipc_namespace *ns, int id)
{
struct kern_ipc_perm *ipcp = ipc_lock(&sem_ids(ns), id);

if (IS_ERR(ipcp))
return (struct sem_array *)ipcp;

return container_of(ipcp, struct sem_array, sem_perm);
}

Expand All @@ -200,6 +208,9 @@ static inline struct sem_array *sem_lock_check(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_check(&sem_ids(ns), id);

if (IS_ERR(ipcp))
return (struct sem_array *)ipcp;

return container_of(ipcp, struct sem_array, sem_perm);
}

Expand Down
20 changes: 17 additions & 3 deletions ipc/shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ int shm_init_ns(struct ipc_namespace *ns)
void shm_exit_ns(struct ipc_namespace *ns)
{
struct shmid_kernel *shp;
struct kern_ipc_perm *perm;
int next_id;
int total, in_use;

Expand All @@ -119,10 +120,11 @@ void shm_exit_ns(struct ipc_namespace *ns)
in_use = shm_ids(ns).in_use;

for (total = 0, next_id = 0; total < in_use; next_id++) {
shp = idr_find(&shm_ids(ns).ipcs_idr, next_id);
if (shp == NULL)
perm = idr_find(&shm_ids(ns).ipcs_idr, next_id);
if (perm == NULL)
continue;
ipc_lock_by_ptr(&shp->shm_perm);
ipc_lock_by_ptr(perm);
shp = container_of(perm, struct shmid_kernel, shm_perm);
do_shm_rmid(ns, shp);
total++;
}
Expand All @@ -149,6 +151,9 @@ static inline struct shmid_kernel *shm_lock_down(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_down(&shm_ids(ns), id);

if (IS_ERR(ipcp))
return (struct shmid_kernel *)ipcp;

return container_of(ipcp, struct shmid_kernel, shm_perm);
}

Expand All @@ -158,6 +163,9 @@ static inline struct shmid_kernel *shm_lock_check_down(
{
struct kern_ipc_perm *ipcp = ipc_lock_check_down(&shm_ids(ns), id);

if (IS_ERR(ipcp))
return (struct shmid_kernel *)ipcp;

return container_of(ipcp, struct shmid_kernel, shm_perm);
}

Expand All @@ -169,6 +177,9 @@ static inline struct shmid_kernel *shm_lock(struct ipc_namespace *ns, int id)
{
struct kern_ipc_perm *ipcp = ipc_lock(&shm_ids(ns), id);

if (IS_ERR(ipcp))
return (struct shmid_kernel *)ipcp;

return container_of(ipcp, struct shmid_kernel, shm_perm);
}

Expand All @@ -177,6 +188,9 @@ static inline struct shmid_kernel *shm_lock_check(struct ipc_namespace *ns,
{
struct kern_ipc_perm *ipcp = ipc_lock_check(&shm_ids(ns), id);

if (IS_ERR(ipcp))
return (struct shmid_kernel *)ipcp;

return container_of(ipcp, struct shmid_kernel, shm_perm);
}

Expand Down

0 comments on commit b1ed88b

Please sign in to comment.