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

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

[Ahmed S. Darwish]

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 <darwish.07@gmail.com>
---

 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

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

[Chris Wright]

* Ahmed S. Darwish (darwish.07@gmail.com) 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

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

[Casey Schaufler]

--- "Ahmed S. Darwish" <darwish.07@gmail.com> 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 <darwish.07@gmail.com>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 
> 


Casey Schaufler
casey@schaufler-ca.com