[MERGED] osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Harald Welte has submitted this change and it was merged.

Change subject: VLR: fix potential NULL dereference
..


VLR: fix potential NULL dereference

The vlr_subscr_get() can return NULL if its argument is NULL
(which isn't checked for) so before dereferencing it's result
we should check for it.

Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
---
M src/libmsc/gsm_04_08.c
1 file changed, 2 insertions(+), 0 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 995bbf6..f87a4c6 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -3721,8 +3721,10 @@
 struct vlr_subscr *vsub)
 {
struct gsm_subscriber_connection *conn = msc_conn_ref;
+   OSMO_ASSERT(vsub);
OSMO_ASSERT(!conn->vsub);
conn->vsub = vlr_subscr_get(vsub);
+   OSMO_ASSERT(conn->vsub);
conn->vsub->cs.attached_via_ran = conn->via_ran;
 }
 

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: neels 

osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Patch Set 3: Code-Review+2

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

[PATCH] osmo-msc[master]: VLR: fix potential NULL dereference

[Neels Hofmeyr]

Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/6388

to look at the new patch set (#3).

VLR: fix potential NULL dereference

The vlr_subscr_get() can return NULL if its argument is NULL
(which isn't checked for) so before dereferencing it's result
we should check for it.

Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
---
M src/libmsc/gsm_04_08.c
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/88/6388/3

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 995bbf6..f87a4c6 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -3721,8 +3721,10 @@
 struct vlr_subscr *vsub)
 {
struct gsm_subscriber_connection *conn = msc_conn_ref;
+   OSMO_ASSERT(vsub);
OSMO_ASSERT(!conn->vsub);
conn->vsub = vlr_subscr_get(vsub);
+   OSMO_ASSERT(conn->vsub);
conn->vsub->cs.attached_via_ran = conn->via_ran;
 }
 

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 3
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: neels 

osmo-msc[master]: VLR: fix potential NULL dereference

[Neels Hofmeyr]

Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/6388/1/src/libmsc/gsm_04_08.c
File src/libmsc/gsm_04_08.c:

Line 3718:  conn->vsub = vlr_subscr_get(vsub);
> a NULL return is only possible if 'vsub' is NULL.  I don't think this is va
agreed


-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-Reviewer: neels 
Gerrit-HasComments: Yes

osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Harald Welte has restored this change.

Change subject: VLR: fix potential NULL dereference
..


Restored

Wrong abandon

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: restore
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 

[ABANDON] osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Harald Welte has abandoned this change.

Change subject: VLR: fix potential NULL dereference
..


Abandoned

Please don't spend time on tghis

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 

osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Patch Set 1:

Heels?

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-HasComments: No

osmo-msc[master]: VLR: fix potential NULL dereference

[Harald Welte]

Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/#/c/6388/1/src/libmsc/gsm_04_08.c
File src/libmsc/gsm_04_08.c:

Line 3718:  conn->vsub = vlr_subscr_get(vsub);
a NULL return is only possible if 'vsub' is NULL.  I don't think this is valid 
here.  Read the comment above the function.  So if at all, I would put an 
OSMO_ASSERT(vsub) here. Neels?


-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels 
Gerrit-HasComments: Yes

[PATCH] osmo-msc[master]: VLR: fix potential NULL dereference

[Max]

Review at  https://gerrit.osmocom.org/6388

VLR: fix potential NULL dereference

The vlr_subscr_get() can return NULL if its argument is NULL
(which isn't checked for) so before dereferencing it's result
we should check for it.

Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
---
M src/libmsc/gsm_04_08.c
1 file changed, 2 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/88/6388/1

diff --git a/src/libmsc/gsm_04_08.c b/src/libmsc/gsm_04_08.c
index 7b6e94a..ac15d18 100644
--- a/src/libmsc/gsm_04_08.c
+++ b/src/libmsc/gsm_04_08.c
@@ -3716,7 +3716,8 @@
struct gsm_subscriber_connection *conn = msc_conn_ref;
OSMO_ASSERT(!conn->vsub);
conn->vsub = vlr_subscr_get(vsub);
-   conn->vsub->cs.attached_via_ran = conn->via_ran;
+   if (conn->vsub)
+   conn->vsub->cs.attached_via_ran = conn->via_ran;
 }
 
 /* operations that we need to implement for libvlr */

-- 
To view, visit https://gerrit.osmocom.org/6388
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I13632908d0b67323202effa9dd6f29732a12cc91
Gerrit-PatchSet: 1
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Owner: Max