Segmentation fault when debugging in C

Dear all,

Currently I am implementing ECC protocols. I used tinyECC package to port ecc to contiki os. This giving me a segmentation fault.

I followed some article Debugging Segmentation Faults and Pointer Problems - Cprogramming.com but I failed to detect the error. Please help me to find that error. Here is my code(part of a code). Error happens in NN_Encode function.

this is some errors occur when debugging

Program received signal SIGSEGV, Segmentation fault.
0x080484c4 in ecc_point2octet (octet=0x80485d0 "UWVS\350i", octet_len=73 'I',
P=0x804a040, compress=0) at octtest.c:108
108 octet[0] = 0x04;

(gdb) info s
#0 0x080484c4 in ecc_point2octet (octet=0x80485d0 "UWVS\350i",
octet_len=73 'I', P=0x804a040, compress=0) at octtest.c:108
#1 0x080485c5 in main () at octtest.c:133

#include <stdio.h>
#include <stdint.h>
typedef uint32_t NN_DIGIT;
typedef uint64_t NN_DOUBLE_DIGIT;

/* Types for length */
typedef uint8_t NN_UINT;
typedef uint16_t NN_UINT2;

#if defined (SECP128R1) || defined (SECP128R2)
#define KEY_BIT_LEN 128
#else 
#if defined (SECP160K1) || defined (SECP160R1) || defined (SECP160R2) 
#define KEY_BIT_LEN 160
#else 
#if defined (SECP192K1) || defined (SECP192R1)
#define KEY_BIT_LEN 192
#else
#define KEY_BIT_LEN 128
#endif /* 192 */
#endif /* 160 */
#endif /* 128 */

/* Length of digit in bits */
#define NN_DIGIT_BITS 32

/* Length of digit in bytes */
#define NN_DIGIT_LEN (NN_DIGIT_BITS/8)

/* Maximum value of digit */
#define MAX_NN_DIGIT 0xffffffff

/* Number of digits in key
 * used by optimized mod multiplication (ModMultOpt) and optimized mod square (ModSqrOpt)
 *
 */
#define KEYDIGITS (KEY_BIT_LEN/NN_DIGIT_BITS) //5

/* Maximum length in digits */
#define MAX_NN_DIGITS (KEYDIGITS+1)

/* buffer size
 *should be large enough to hold order of base point
 */
#define NUMWORDS MAX_NN_DIGITS

/* the mask for ModSqrOpt */
#define MOD_SQR_MASK1 0x8000000000000000ll
#define MOD_SQR_MASK2 0x0000000100000000ll

typedef struct Point
{
    // point's coordinates
    NN_DIGIT x[NUMWORDS];
    NN_DIGIT y[NUMWORDS];
} Point;

Point pbkey_alice;
void 
NN_Encode(unsigned char *a, NN_UINT len, NN_DIGIT *b, NN_UINT digits)
{
  NN_DIGIT t;
  int j;
  unsigned int i, u;

  for(i = 0, j = len - 1; i < digits && j >= 0; i++) {
    t = b;
    for(u = 0; j >= 0 && u < NN_DIGIT_BITS; j--, u += 8) {
      a[j] = (unsigned char)(t >> u);
    }
  }

  for(; j >= 0; j--) {
    a[j] = 0;
  }
}

int ecc_point2octet(uint8_t *octet, NN_UINT octet_len, Point *P, int compress)
{
	if (compress){
		if(octet_len < KEYDIGITS*NN_DIGIT_LEN+1)
	  {//too small octet
		return -1;
    }
    else
    {
		  //compressed point representation
			if((1 & P->y[0]) == 0){
	  	octet[0] = 0x02;
			}
			else
				{
	  			octet[0] = 0x03;
				}
			NN_Encode(octet+1, KEYDIGITS*NN_DIGIT_LEN, P->x, KEYDIGITS);
			return KEYDIGITS*NN_DIGIT_LEN+1;
      }
	}
  else
  {//non compressed
	 if(octet_len < 2*KEYDIGITS*NN_DIGIT_LEN+1)
    {
    
		return -1;
    }
    else
    {
			octet[0] = 0x04;
			NN_Encode(octet+1, KEYDIGITS*NN_DIGIT_LEN, P->x, KEYDIGITS);
			NN_Encode(octet+1+KEYDIGITS*NN_DIGIT_LEN, KEYDIGITS*NN_DIGIT_LEN, P->y, KEYDIGITS);
			return 2*KEYDIGITS*NN_DIGIT_LEN+1;
		}
  }
}

int main()
{
pbkey_alice.x[5] = 0x00000000;
  pbkey_alice.x[4] = 0x21961f69;
  pbkey_alice.x[3] = 0xf02d202b;
  pbkey_alice.x[2] = 0xa4b41f1a;
  pbkey_alice.x[1] = 0x0aa08a86;
  pbkey_alice.x[0] = 0xdf27908d;
    
  pbkey_alice.y[5] = 0x00000000;
  pbkey_alice.y[4] = 0x378e1278;
  pbkey_alice.y[3] = 0x62836d75;
  pbkey_alice.y[2] = 0x7acb7ca4;
  pbkey_alice.y[1] = 0x0dc0ad13;
  pbkey_alice.y[0] = 0x741e287c;
uint8_t *C;
int C_len = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20;
int oct_len = ecc_point2octet(C, C_len, &pbkey_alice, 0);

}

Let's start in main(). You initialize six elements of two arrays each of which are allocated five elements. (pbkey_alice is an object of type Point which is defined by:

typedef struct Point
{
    // point's coordinates
    NN_DIGIT x[NUMWORDS];
    NN_DIGIT y[NUMWORDS];
} Point;

Point pbkey_alice;

which after one round of evaluating macros expands to:

typedef struct Point
{
    // point's coordinates
    uint32_t x[MAX_NN_DIGITS];
    uint32_t y[MAX_NN_DIGITS];
} Point;

Point pbkey_alice;

and then MAX_NN_DIGITS which expands to (KEYDIGITS+1) which expands to ((KEY_BIT_LEN/NN_DIGIT_BITS)+1) (with a comment saying that KEY_BIT_LEN/NN_DIGIT_BITS is 5) which (since none of the following are defined: SECP128R1, SECP128R2, SECP160K1, SECP160R1, SECP160R2, SECP192K1, and SECP192R1) expands to ((128/32)+1) (note that 128/32 is 4; not 5) which finally evaluates to:

]typedef struct Point
{
    // point's coordinates
    uint32_t x[5];
    uint32_t y[5];
} Point;

Point pbkey_alice;

If you start off in main() writing into unallocated areas, you should expect that something bad is going to happen.

Then you allocate a pointer uint8_t *C; but do not initialize it. You then pass this uninitialized pointer to ecc_point2octet() which then stores data into the byte pointed to by this uninitialized pointer and then calls NN_Encode() to store more data into subsequent bytes using the same uninitialized pointer. Storing data into areas pointed to by uninitialized (or corrupted) pointers is a very common way to get a segmentation fault.

You're on your own from here.

First of all thanks very much for your explanation, I did most of them blindly. I initialized C and it works now.
uint8_t *C = malloc(sizeof(uint8_t));

thanks in advance

That is not sufficient. You need at least (C_len * sizeof(uint8_t)) bytes, and C_len isn't defined yet where you're initializing C.

Did you fix the fact that you're putting more data into pbkey_alice than it is allocated to hold?

in actual case I am using KEY_BIT_LEN as 160, and I used C_len like this
C_len = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20; and use it in function,,,something wrong with it?

and I can't understand "Did you fix the fact that you're putting more data into pbkey_alice than it is allocated to hold?" I think I'm not. could you explain it to me?

As I explained in my first message on this thread, the source you showed us sets KEY_BIT_LEN to 128; not to 160.

You said you initialize C using the statement:

uint8_t *C = malloc(sizeof(uint8_t));

which initializes C to be a pointer to a buffer with space to hold 1 byte.
You then pass that pointer to

ecc_point2octet(C, C_len, &pbkey_alice, 0);

telling it that C is a pointer to a buffer containing 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20 bytes.

Do you not see that there is a problem when you tell ecc_point2octet() you are passing it a pointer to a buffer containing 73 bytes when you pass it a pointer to a buffer that has only been allocated 1 byte?

yeah,,something wrong, I'm getting an another error

---------- Post updated at 09:59 PM ---------- Previous update was at 09:37 PM ----------

in nesc code it use like this
uint8_t C[2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20];

so I used like this,,,can we use like this? 73* 1 bytes?
int xxx = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20+ 20;
uint8_t C = malloc(xxxsizeof(uint8_t));

So, you're getting another error. If you want help, you need to tell us what the error is and give us a way to track it down. This is your program we're trying to debug. Most of us don't have access to your system (or any system running contiki os) and don't have (or want) tinyECC packages on our systems. The comments I've given you are just based on reading your code, knowing what generates a segmentation fault, and a few decades of experience writing C programs.

As to your last comment. If I understand correctly, you now have the following four lines of code in your source:

int xxx = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20+ 20;
uint8_t *C = malloc(xxx*sizeof(uint8_t));
int C_len = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20;
int oct_len = ecc_point2octet(C, C_len, &pbkey_alice, 0);

Wouldn't it make more sense to replace this with:

int C_len = 2*KEYDIGITS*NN_DIGIT_LEN + 1 + 20 + 20;
uint8_t *C = malloc(C_len*sizeof(uint8_t));
int oct_len = ecc_point2octet(C, C_len, &pbkey_alice, 0);

(since xxx is a duplicate of C_len) or, if C_len isn't used anywhere else, with:

uint8_t *C = malloc((2*KEYDIGITS*NN_DIGIT_LEN+1+20+20)*sizeof(uint8_t));
int oct_len = ecc_point2octet(C, sizeof(C), &pbkey_alice, 0);

thanks for your advice and did up to some level.

In my case I have to deal with 3 different parties. TinyECC(nesc), native c and contiki. What I am doing is convert nesc in to c then use it in contiki. And I don't have much knowledge in c. . I can't even found it's a segmentation fault or what in contiki. ( they call contiki as embeded os and we don't need to install it, only download package and go in to it and run simulator using 'ant run'). So I extract some codes out and create above source file to get the error.( I don't know way to debug in cooja simulator in contiki) .

But with your help I ported most of codes and correct errors. And I need to check my code is doing right things,,,if you don't mind please look at this code and give me idea how to compare two unsigned integers

  static void bacast_signed_message()
    {
      uint8_t *M = malloc(MAX_M_LEN*sizeof(uint8_t));//uint8_t M[MAX_M_LEN];//uint8_t *M;
      int M_len = MAX_M_LEN;;
      
      uint8_t *C = malloc((2*KEYDIGITS*NN_DIGIT_LEN + 1 + MAX_M_LEN + HMAC_LEN)*sizeof(uint8_t));	
      int C_len; 
      //uint8_t C[2*KEYDIGITS*NN_DIGIT_LEN + 1 + MAX_M_LEN + HMAC_LEN]; 
      
      uint8_t *dM = malloc(MAX_M_LEN*sizeof(uint8_t));  //uint8_t dM[MAX_M_LEN];
      int dM_len = MAX_M_LEN;
      
      random_data(M, MAX_M_LEN);
    
      printf("C before encrypt %p\n",*C);
      printf("M before encrypt %p\n",*M);
      printf("dM before encrypt %p\n",*dM);
     	
      C_len = encrypt(C, (2*KEYDIGITS*NN_DIGIT_LEN + 1 + M_len + HMAC_LEN), M, M_len, &pbkey_alice);
      //encrypt(uint8_t *C, int C_len, uint8_t *M, int M_len, Point *PublicKey);
      
      printf("C after encrypt %p\n",*C);
      printf("M after encrypt %p\n",*M);
      printf("dM after encrypt %p\n",*dM);
    	
      dM_len = decrypt(dM, dM_len, C, C_len, prKey_alice);   
      //decrypt(uint8_t *M, int M_len, uint8_t *C, int C_len, NN_DIGIT *d);
      
      printf("C after decrypt %p\n",*C);
      printf("M after decrypt %p\n",*M);
      printf("dM after decrypt %p\n",*dM);
    	
      printf("C_len = %i , M_len = %i\n",C_len,M_len);
      
      if (dM == M){printf("Works\n");}
      else{printf("Not Works\n");}
    }

and this is the out put I got

  C before encrypt 0x40
    M before encrypt 0x28
    dM before encrypt 0x70
    C after encrypt 0x4
    M after encrypt 0x28
    dM after encrypt 0x70
    C after decrypt 0x4
    M after decrypt 0x28
    dM after decrypt 0x28
    C_len = 102 , M_len = 41
    Not Works

And if I changed private_key(no change of public key) then It looks like this

  C before encrypt 0x40
    M before encrypt 0x28
    dM before encrypt 0x70
    C after encrypt 0x4
    M after encrypt 0x28
    dM after encrypt 0x70
    C after decrypt 0x4
    M after decrypt 0x28
    dM after decrypt 0x70
    C_len = 102 , M_len = 41
    Not Works

That mean dM has not changed no. But I am not sure about this. If I use %u instead of %p it will give me totally different answers (it says if we use %u it'll convert to decimal. Please give me some advice, and tell me wrong whats with.

Thanks in advance sorry for bother you.

You are not bothering me. It is just hard to help when there isn't much debugging data to help analyze the problems. :wall: However, the output you've provided helps. It is also clear from the debugging code you've added and the output you've displayed that you are having problems understanding how pointers work in C. Let me walk you through some of your code and explain what it means.

First, the declarations:

uint8_t *M = malloc(MAX_M_LEN*sizeof(uint8_t));
uint8_t *C = malloc((2*KEYDIGITS*NN_DIGIT_LEN + 1 + MAX_M_LEN + HMAC_LEN)*sizeof(uint8_t));	
uint8_t *dM = malloc(MAX_M_LEN*sizeof(uint8_t));

create M, C, and dM as pointers to objects of type uint8_t and the calls to malloc() allocate buffers to hold data to be stored into the areas pointed to by those pointers. Then you have:

printf("C before encrypt %p\n",*C);
printf("M before encrypt %p\n",*M);
printf("dM before encrypt %p\n",*dM);

The %p in these printf format strings are used to convert the value of a pointer (the address to which the pointer points) to a printable value. But the operands you're providing for these conversions are objects of type uint8_t, not pointers. (C is a pointer, *C is the uint8_t object pointed to by that pointer; M is a pointer, *M is the uint8_t object pointed to by M; dM is a pointer, *dM is the uint8_t object pointed to by dM.) If you want to print the contents of the pointers, the above code needs to be changed to:

printf("C before encrypt %p\n",C);
printf("M before encrypt %p\n",M);
printf("dM before encrypt %p\n",dM);

If you want to print the unsigned byte pointed to be these pointers (both as an unsigned decimal value and as a printable character), the above code needs to be changed to something like:

printf("*C before encrypt %hhu(%c)\n",*C,*C);
printf("M before encrypt %hhu(%c)\n",*M,*M);
printf("dM before encrypt %hhu(%c)\n",*dM,*dM);

Then your code:

if (dM == M){printf("Works\n");}
else{printf("Not Works\n");}

is comparing the pointers (not the unsigned 8-bit integers to which they point). Since these pointers were initialized by separate calls to malloc() and not changed since they were initialized, they can't have the same value. They might point to bytes that have the same value, but that would be coded as:

if (*dM == *M){printf("Works\n");}
else{printf("Not Works\n");}

or as:

printf("%sWorks\n", (*dM == *M) ? "" : "Not ");

While we're talking about pointers, you may also see an ampersand followed by an object (i.e., &object ). This gives you a pointer to the object. So, for example, the following code segment:

char *ptr = "abcd";
printf("&ptr=%p, ptr=%p, *p=%c\n", &ptr, ptr, *ptr);

will print the address in memory where ptr is allocated (&ptr), the address in memory of the first byte in the string "abcd" (ptr), and the character pointed to by ptr (*ptr) which in this case is 'a'.

I hope this helps.

To elaborate what DonCragun explained:

A pointer is a memory address where a certain object is stored. Suppose the line:

x = "abcd"

Most compilers (not only C-compilers) will do something similar to this: first, allocate 4 bytes to hold "abcd", then label this 4-byte-part of the memory with "x". Subsequent usages of "x" will be dereferenced to this location.

Now suppose you want to pass this variable to a function, but you want the function to modify it somehow. If you just pass the variable as an argument the compiler creates a copy of the data and the function will use this copy. Changes made to this copy are naturally lost once the function ends. Therefore C can pass a pointer instead of the variable itself. By passing the pointer the function gains access not to the copy, but the original and can modify it lastingly.

It is also possible to find out the memory address of an object (the "&" operator DonCragun told you about) and it is possible to get the variable from a pointer address: the "*" operator.

Now, all pointers are of the same format - addresses in memory - and this begs the question why C pointers have types. Regardless of the types all pointers would look the same, no? Yes and no: yes, they look the same. No, they need to be typed because pointers in C are "intelligent": as they know which data they point to, it is possible to use pointer arithmetics.

Suppose you have an array of 32-bit-values. When the program runs this is a series of 4-byte long memory-locations. Now suppose you have a pointer to the first element. If you create a pointer in C and tell it the correct data type the pointer will "know" that the data "unit" it points to is 4 bytes long. an operation like ++ on the pointer will then not increment the address it points to by one (which would be the address of the second byte of the same unit - rather senseless) but by 4, so that the pointer now points to the next units first byte. This way it is easy to go from one to the next element of the array. This would not be possible if your pointer wouldn't know which size the data it points to is.

I hope this helps.

bakunin

thanks both of you,