Skip to contentSkip to conversation

Smack: Do not dereference NULL ipc object

PATCH In review Ahmed S. Darwish 3 participants 3 messages Mar 14, 2008 – Mar 20, 2008

Subsystems (2)
Security (LSM) Smack security module
lore
Share & tools
lore

Ahmed S. Darwish

author
#0 lore raw

PATCH 1/1 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(-)
security/smack/smack_lsm.c +6 -3
index 0241fd3..38d7075 100644
@@ -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,

Signature · 5 lines
"Better to light a candle, than curse the darkness"

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com
#1 lore raw
Quoted 6 lines · * 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

maintainer Acked-by on v1 1/1
#2 lore raw
In reply to Ahmed S. Darwish
Quoted 10 lines · --- "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 <[email protected]>
Acked-by Casey Schaufler <[email protected]>

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

Quoted 66 lines

---

 smack_lsm.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
security/smack/smack_lsm.c +1 -1
index 0241fd3..38d7075 100644
@@ -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;
security/smack/smack_lsm.c +2 -1
 	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;
security/smack/smack_lsm.c +2 -1
 	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;
security/smack/smack_lsm.c +1 -0
 	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);
 }
Quoted 18 lines

Regards,

Signature · 11 lines
"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]