2008-03-14 23:14:19

by Ahmed S. Darwish

[permalink] [raw]
Subject: [PATCH BUGFIX -rc5] Smack: Do not dereference NULL ipc object

Hi all,

In the SYSV ipc msgctl(),semctl(),shmctl() family, if the user passed
*_INFO as the desired operation, no specific object is meant to be
controlled and only system-wide information is returned. This leads
to a NULL IPC object in the LSM hooks if the _INFO flag is given.

Avoid dereferencing this NULL pointer in Smack ipc *ctl() methods.

Signed-off-by: Ahmed S. Darwish <[email protected]>
---

smack_lsm.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 0241fd3..38d7075 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1508,7 +1508,7 @@ static int smack_shm_associate(struct shmid_kernel *shp, int shmflg)
*/
static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
{
- char *ssp = smack_of_shm(shp);
+ char *ssp;
int may;

switch (cmd) {
@@ -1532,6 +1532,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
return -EINVAL;
}

+ ssp = smack_of_shm(shp);
return smk_curacc(ssp, may);
}

@@ -1616,7 +1617,7 @@ static int smack_sem_associate(struct sem_array *sma, int semflg)
*/
static int smack_sem_semctl(struct sem_array *sma, int cmd)
{
- char *ssp = smack_of_sem(sma);
+ char *ssp;
int may;

switch (cmd) {
@@ -1645,6 +1646,7 @@ static int smack_sem_semctl(struct sem_array *sma, int cmd)
return -EINVAL;
}

+ ssp = smack_of_sem(sma);
return smk_curacc(ssp, may);
}

@@ -1730,7 +1732,7 @@ static int smack_msg_queue_associate(struct msg_queue *msq, int msqflg)
*/
static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
{
- char *msp = smack_of_msq(msq);
+ char *msp;
int may;

switch (cmd) {
@@ -1752,6 +1754,7 @@ static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
return -EINVAL;
}

+ msp = smack_of_msq(msq);
return smk_curacc(msp, may);
}

Regards,

--

"Better to light a candle, than curse the darkness"

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com


2008-03-20 00:02:04

by Chris Wright

[permalink] [raw]
Subject: Re: [PATCH BUGFIX -rc5] Smack: Do not dereference NULL ipc object

* Ahmed S. Darwish ([email protected]) wrote:
> In the SYSV ipc msgctl(),semctl(),shmctl() family, if the user passed
> *_INFO as the desired operation, no specific object is meant to be
> controlled and only system-wide information is returned. This leads
> to a NULL IPC object in the LSM hooks if the _INFO flag is given.
>
> Avoid dereferencing this NULL pointer in Smack ipc *ctl() methods.

Casey, this looks quite legit to me. Can you ack please.

thanks,
-chris

2008-03-20 00:13:11

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH BUGFIX -rc5] Smack: Do not dereference NULL ipc object


--- "Ahmed S. Darwish" <[email protected]> wrote:

> Hi all,
>
> In the SYSV ipc msgctl(),semctl(),shmctl() family, if the user passed
> *_INFO as the desired operation, no specific object is meant to be
> controlled and only system-wide information is returned. This leads
> to a NULL IPC object in the LSM hooks if the _INFO flag is given.
>
> Avoid dereferencing this NULL pointer in Smack ipc *ctl() methods.
>
> Signed-off-by: Ahmed S. Darwish <[email protected]>

Acked-by: Casey Schaufler <[email protected]>

Sorry, somehow I thought I'd acked this some time ago.

> ---
>
> smack_lsm.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 0241fd3..38d7075 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1508,7 +1508,7 @@ static int smack_shm_associate(struct shmid_kernel
> *shp, int shmflg)
> */
> static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
> {
> - char *ssp = smack_of_shm(shp);
> + char *ssp;
> int may;
>
> switch (cmd) {
> @@ -1532,6 +1532,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp,
> int cmd)
> return -EINVAL;
> }
>
> + ssp = smack_of_shm(shp);
> return smk_curacc(ssp, may);
> }
>
> @@ -1616,7 +1617,7 @@ static int smack_sem_associate(struct sem_array *sma,
> int semflg)
> */
> static int smack_sem_semctl(struct sem_array *sma, int cmd)
> {
> - char *ssp = smack_of_sem(sma);
> + char *ssp;
> int may;
>
> switch (cmd) {
> @@ -1645,6 +1646,7 @@ static int smack_sem_semctl(struct sem_array *sma, int
> cmd)
> return -EINVAL;
> }
>
> + ssp = smack_of_sem(sma);
> return smk_curacc(ssp, may);
> }
>
> @@ -1730,7 +1732,7 @@ static int smack_msg_queue_associate(struct msg_queue
> *msq, int msqflg)
> */
> static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
> {
> - char *msp = smack_of_msq(msq);
> + char *msp;
> int may;
>
> switch (cmd) {
> @@ -1752,6 +1754,7 @@ static int smack_msg_queue_msgctl(struct msg_queue
> *msq, int cmd)
> return -EINVAL;
> }
>
> + msp = smack_of_msq(msq);
> return smk_curacc(msp, may);
> }
>
> Regards,
>
> --
>
> "Better to light a candle, than curse the darkness"
>
> Ahmed S. Darwish
> Homepage: http://darwish.07.googlepages.com
> Blog: http://darwish-07.blogspot.com
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
>


Casey Schaufler
[email protected]