Project

General

Profile

Bug #1640

Missing release field/length in MS RA capability container

Added by prasadkg about 5 years ago. Updated 6 months ago.

Status:
Feedback
Priority:
Low
Assignee:
-
Target version:
-
Start date:
03/10/2016
Due date:
% Done:

10%

Spec Reference:

Description

Description
In case of MS RA Capability container "Content_t", there is no dedicated field to denote the existence of 3GPP release in the message. The encoder goes out of max length allowed for release supported and encodes incorrectly.
Example vector with failure:
vector1 = 40165e000000268ca2a050740440000000300b2b2b2b2b
vector2 = 40165e00000026d0a2a0507400000220000000180b2b2b
vector1 == vector2 : FALSE

Proposal:
For this, we shall have precomputed value of length to terminate the container encoding before calling Content_Dissector in csn1.cpp file. The pre computing length logic routine shall consider the presence of different 3GPP releases.

For example in case of release 5 RA capability message , encoder should be able to code only till release 5 fields . In case of release 6 RA capability, encoder must be able to code till release 6. This release information must be available to the encoder to calculate length. Decoder will set the release field based on the fields present in the message.

Associated revisions

Revision fabf06a3
Added by max over 3 years ago

Fix RAI construction

The gsm48_construct_ra() expect 6-byte buffer while ra_id.digits is
3-byte buffer. The function fills in LAC and RAC as well so we should
pass entire struct, not just 'digits' part which only store MCC/MNC.

Related: OS#1640
Change-Id: I3bfda930012c792452f9fd695ed7acf46365f1df
Fixes: CID57877, CID57876

Revision f1ad60e4 (diff)
Added by max over 3 years ago

Add function to properly encode RAI

Add gsm48_encode_ra() which takes appropriate struct as [out] parameter
instead of generic buffer. Using uint8_t buffer instead of proper struct
type prooved to be error-prone - see Coverity CID57877, CID57876.

Old gsm48_construct_ra() is made into tiny wrapper around new
function. The test output is adjusted because of the change in function
return value which was constant and hence ignored anyway.

Related: OS#1640
Change-Id: I31f9605277f4945f207c2c44ff82e62399f8db74

Revision 309d0e54 (diff)
Added by max over 3 years ago

Deprecate gsm48_construct_ra()

It's just a tiny wrapper around gsm48_encode_ra() with less strict type
signature.

Related OS#1640
Change-Id: I79d6d1133afbf32e891a6b0e3a244c6885ea9614

Revision 9f13b140 (diff)
Added by max about 3 years ago

Use gsm48_encode_ra() for RAI encoding

It has stricter type signature which increase the chance of spotting
misuse either via compiler warning or with automated scan. This also
paves the way for gsm48_construct_ra() deprecation in libosmocore.

Change-Id: I2c0f082dc7214ed57a40dad0788e34b838dfac97
Related: OS#1640

History

#1 Updated by laforge over 4 years ago

the proposal seems fine to me. will your team be working on this?

#2 Updated by arvind.sirsikar over 4 years ago

  • Status changed from New to Stalled

Currently no impact on the test scenarios. Hence low priority.

#3 Updated by laforge over 3 years ago

  • Assignee set to sysmocom

#4 Updated by laforge over 3 years ago

  • Status changed from Stalled to New
  • Assignee changed from sysmocom to msuraev

#5 Updated by laforge over 3 years ago

  • Priority changed from Normal to High

#6 Updated by msuraev about 3 years ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

#7 Updated by msuraev about 3 years ago

  • Status changed from In Progress to Stalled

#8 Updated by laforge about 3 years ago

  • Assignee changed from msuraev to sysmocom

#9 Updated by laforge about 3 years ago

  • Assignee changed from sysmocom to lynxis

#10 Updated by laforge over 2 years ago

  • Priority changed from High to Immediate

#11 Updated by laforge over 2 years ago

  • Priority changed from Immediate to High

#12 Updated by laforge over 2 years ago

#13 Updated by laforge over 2 years ago

  • Assignee changed from lynxis to msuraev

#14 Updated by laforge about 2 years ago

  • Assignee changed from msuraev to lynxis

#15 Updated by laforge over 1 year ago

  • Assignee changed from lynxis to sysmocom

#16 Updated by laforge about 1 year ago

  • Priority changed from High to Low

#17 Updated by laforge 6 months ago

  • Status changed from Stalled to Feedback
  • Assignee changed from sysmocom to fixeria

fixeria, is this still an issue?

#18 Updated by fixeria 6 months ago

  • Assignee deleted (fixeria)

Hi Harald,

fixeria, is this still an issue?

I remember we had some problems with encoding of the MS RA Capability, but this is not critical. Decoding is important for the PCU, because it needs to know the MS capabilities, and it seems to work fine. Encoding is not. The only place where we do encoding is the unit test, so unless I am missing something, it's not worth to spend time on that.

Here is what I get with the current master:

vector1 = 40165e000000268ca2a050740440000000300b2b2b2b2b
=========Start DECODE===========
+++++++++Finish DECODE (0)++++++++++
=========Start ENCODE=============
+++++++++Finish ENCODE (0)+++++++++++
vector1 = 40 16 5e 00 00 00 26 8c a2 a0 50 74 04 40 00 00 00 30 0b 2b 2b 2b 2b 
vector2 = 40 16 5e 00 00 00 27 0c a2 a0 50 74 00 00 00 00 04 40 00 00 00 30 0b 
vector1 == vector2 : FALSE

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)