Segfault... Why?

Okay, so it's been quite a number of years since I've done any C (college?), but I'm a fairly decent programmer in other languages (Java, PHP, Perl, etc.).

I'm writing a C module for AIX to be used when a user changes his/her password. It's referred to as the "Password check method" or "pwdcheck" setting for the system.

I customized example C code for this module to make it do what I wanted (check to make sure the password contains characters from three of the four character types). Here is the code:

Code:

---

#include <string.h>
#include <stdlib.h>
#include <sys/errno.h>
#include <ctype.h>
#define errDiff  "Your password must contain at least one character of 3 of the 4 types: lower, upper, digit and special character\n"

int pwdrestrict_method (UserName, NewPassword, OldPassword, Message)
char  * UserName;
char  * NewPassword;
char  * OldPassword;
char ** Message;
{
 char * localMessage;
 int    charCnt = 0;
 int    i = 0;
 int    lCnt = 0, uCnt = 0, dCnt = 0, pCnt = 0, cTypes = 0;

 charCnt = strlen(NewPassword);
 for (i=0; i<charCnt; i++) {
  if (islower(NewPassword[i]))
  lCnt++;
  else if (isupper(NewPassword[i]))
  uCnt++;
  else if (isdigit(NewPassword[i]))
  dCnt++;
  else if (ispunct(NewPassword[i]))
  pCnt++;
 }

 printf("NewPassword=%s lCnt=%d uCnt=%d dCnt=%d pCnt=%d charCnt=%d\n",NewPassword,lCnt,uCnt,dCnt,pCnt,charCnt);
 cTypes = (lCnt!=0) + (uCnt!=0) + (dCnt!=0) + (pCnt!=0);
 printf("cTypes=%d\n",cTypes);

 if (cTypes < 3) {
  localMessage = (char *) malloc(strlen(errDiff+1));
  strcpy(localMessage, errDiff);
  *Message = localMessage;
  errno = EPERM;
  return 1;
 }

 Message = NULL;
 return 0;
}

---

Now, when I compile this module (cc -e pwdrestrict_method -o pwdrestrict pwdrestrict.c), copy the compiled executable to /usr/lib/pwdrestrict and point a test user's pwdcheck property to it, when I change that user's password, here is the behavior I get:

If the first password typed doesn't meet the requirements (say, "12345"), the error message "errDiff" is displayed to the user and they are re-prompted to choose another password.
If the second password typed meets the requirements, they are prompted to enter the password twice and the password is set successfully.

If the first password typed meets the requirements, I get a segfault and the password prompt exits (obviously).

All subsequent runs of the passwd command "reset" this behavior.

I am using IBM's xlC compiler to compile this code, and I don't know if they have a debugger like the GNU C compiler has (which I don't really know how to use anyway, but I found plenty of how-tos out there for).

So can anyone please help me identify where my problem might be? Or tell me some way to debug this issue?

I've tried adding the debug output (printf) statements in there to watch what the variables are doing, and I've tried commenting out various chunks of code to use process of elimination to determine where the segfault is coming from, but it seems to happen no matter what I leave it or leave out...

Thanks in advance for any and all help!

Regards,
Jon Heese

Don't know what debugger to use with xlC, so I can't help you there. A seg fault results from invalid memory accesses. Typically, this is reading from some place you don't have read permissions, or writing to some place you don't have write permissions, often the result of accessing a null pointer or an uninitialized pointer. You could try compiling the code on a Linux system with GCC and see if you can replicate the seg fault, at use GDB to track it down that way if need be. A full example we can compile and debug might be helpful here, but first:

Some potential problems:

• UserName, NewPassword, OldPassword or Message are null or contain invalid addresses, so dereferencing them, indexing them, calling strlen on them, etc could be the result. This depends on how they're declared in the calling function and how you pass them in.
• Message may not be null, but The address it points to may be incorrect, so things like *Message may result in a seg fault.
• malloc may return NULL if it fails, so the strcpy into localMessage may seg fault.

Also, when printing debug output, it's best to use fprintf to print to stderr, since stdout is line buffered, and the line isn't always printed immediately, so it can give misleading information as to where your program was when it crashed.

Great, thanks for the tips! I will definitely look into that.

I have verified that UserName, OldPassword, and NewPassword are valid and not null, but my program doesn't use UserName or OldPassword at all, so it seems like they can just be ignored in terms of this segfault, right?

How can I check whether Message contains an invalid memory reference/address? As I said in my OP, I have commented out just about the entire code (including every reference to Message), and still got the segfault, so I don't think the problem is there.

Maybe it would be helpful to include the original example code (which does *not* display the segfault behavior) that I built my code from:

Code:

---

#include <string.h>
#include <stdlib.h>
#include <sys/errno.h>
#include <ctype.h>

#define errIdent "  Try again, password cannot be the same as login\n"
#define errChng  "  Try again, new password cannot be same as old password\n"
#define errDiff  "  Try again, with at least one lower, upper & digit character\n"

int pwdrestrict_method (UserName, NewPassword, OldPassword, Message)
char  * UserName;
char  * NewPassword;
char  * OldPassword;
char ** Message;
{
 char * localMessage;
 int    charCnt;
 int    i;
 int    lCnt = 0, uCnt = 0, dCnt = 0;

 if (strcmp(UserName, NewPassword) == 0) {    /* UserName = Password      */
  localMessage = (char *) malloc(strlen(errIdent)+1); /* +1 for \0 at end    */
  strcpy(localMessage, errIdent);
  *Message = localMessage;
  errno = EPERM;                /* Operation not permitted  */
  return 1;
 }
 
 if (strcmp(NewPassword, OldPassword) == 0) {    /* OldPassword = NewPassword */
  localMessage = (char *) malloc(strlen(errChng)+1);
  strcpy(localMessage, errChng);
  *Message = localMessage;
  errno = EPERM;                /* Operation not permitted  */
  return 1;
 }

 charCnt = strlen(NewPassword);
 for (i=0; i<charCnt; i++) {
  if (islower(NewPassword[i]))
  lCnt++;
  else if (isupper(NewPassword[i]))
  uCnt++;
  else if (isdigit(NewPassword[i]))
  dCnt++;
 }
 if (lCnt == 0 || uCnt == 0 || dCnt == 0) {
  localMessage = (char *) malloc(strlen(errChng)+1);
  strcpy(localMessage, errDiff);
  *Message = localMessage;
  errno = EPERM;                /* Operation not permitted  */
  return 1;
 }

 Message = NULL;
 return 0;
}

---

Since this is a module that plugs into existing AIX OS-level framework, I'm not sure how I could compile and test this on Linux with GCC, but I guess I could convert it to a standalone C program with a main method that takes arguments from the invoker, right? Can you help me do that? I understand the desire to make folks do their own work to learn, so I will attempt it if you want me to, but keep in mind that I'm not very C-capable, so I might come back with something that looks totally ridiculous. :)

Thanks again!

Regards,
Jon Heese

Code:

---

localMessage = (char *) malloc(strlen(errChng)+1);
strcpy(localMessage, errDiff);

---

Typo? That could cause a segfault since you are allocating less memory than you need to hold the string being copied - strlen(errChng) is less than strlen(errDiff). But that's in the code you say works.

In general:
1 - What's with the K&R style function parameters? You're probably in the 0.00000001% of programmers that use that style anymore.
2 - Don't cast malloc, the void* pointer returned can be safely converted/assigned without the need for a cast.

Quote:

---

Originally Posted by hk_mp5kpdw

1 - What's with the K&R style function parameters? You're probably in the 0.00000001% of programmers that use that style anymore.

---

It's the insanity of many developers who want the code to compile even in pre-standard compilers! :mad:

Quote:

---

Originally Posted by hk_mp5kpdw

Code:

---

localMessage = (char *) malloc(strlen(errChng)+1);
strcpy(localMessage, errDiff);

---

Typo? That could cause a segfault since you are allocating less memory than you need to hold the string being copied - strlen(errChng) is less than strlen(errDiff). But that's in the code you say works.

---

Yes, I noticed that typo as well, and I fixed it in my code. I think the original author of that code didn't actually test the code before a last-minute modification before posting. It was from here:http://www.ibm.com/developerworks/fo...ageID=14290884 I don't exactly know how, but I can guarantee that the code I posted in post #3 works exactly as posted and does not segfault when the errDiff message is triggered.

Quote:

---

In general:
1 - What's with the K&R style function parameters? You're probably in the 0.00000001% of programmers that use that style anymore.

---

I didn't write that part of the code, I just left it the way it was and added/removed the bits I did/didn't need.

Quote:

---

2 - Don't cast malloc, the void* pointer returned can be safely converted/assigned without the need for a cast.

---

Okay, I understand the jist of what you said there, but I'm not C-savvy enough to draw a conclusion of what it actually means in terms of the code...

Are you suggesting?:

Code:

---

localMessage = malloc(strlen(errDiff+1));

---

Thanks.

Regards,
Jon Heese

Quote:

---

Originally Posted by jonheese

Okay, I understand the jist of what you said there, but I'm not C-savvy enough to draw a conclusion of what it actually means in terms of the code...

Are you suggesting?:

Code:

---

localMessage = malloc(strlen(errDiff+1));

---

---

Yes, that is what's being suggested. Sounds like you're more C-savvy than you think :).

Quote:

---

Originally Posted by jonheese

Since this is a module that plugs into existing AIX OS-level framework, I'm not sure how I could compile and test this on Linux with GCC, but I guess I could convert it to a standalone C program with a main method that takes arguments from the invoker, right? Can you help me do that? I understand the desire to make folks do their own work to learn, so I will attempt it if you want me to, but keep in mind that I'm not very C-capable, so I might come back with something that looks totally ridiculous. :)

---

Yes, a standalone program that demonstrates the seg fault would be good. I'm feeling generous, so to get you started:

Code:

---

#include <stdio.h>  // for things like printf, scanf, fgets
#include <string.h>  // for things like strlen, strstr, etc
#include <ctype.h>  // for things like islower, isupper

#define MAX_LEN    64  // or some other suitable size

int main(void)
{
    char old[MAX_LEN], new[MAX_LEN];  // maybe you need other variables like username too

    printf("Enter old password: ");
    fgets(old, sizeof(old), stdin);
...
    return 0;
}

---

Quote:

---

Originally Posted by jonheese

Are you suggesting?:

Code:

---

localMessage = malloc(strlen(errDiff+1));

---

Thanks.

Regards,
Jon Heese

---

My guess is below; note did not read thread.
Tim S.

Code:

---

localMessage = malloc(strlen(errDiff)+1);

---

Quote:

---

Originally Posted by anduril462

Yes, a standalone program that demonstrates the seg fault would be good. I'm feeling generous, so to get you started:

Code:

---

#include <stdio.h>  // for things like printf, scanf, fgets
#include <string.h>  // for things like strlen, strstr, etc
#include <ctype.h>  // for things like islower, isupper

#define MAX_LEN    64  // or some other suitable size

int main(void)
{
    char old[MAX_LEN], new[MAX_LEN];  // maybe you need other variables like username too

    printf("Enter old password: ");
    fgets(old, sizeof(old), stdin);
...
    return 0;
}

---

---

Great, thanks!

Regards,
Jon Heese

Can you show how you're calling this (if possible)? It could perhaps be

Code:

---

Message = NULL;

---

, which should probably be

Code:

---

*Message = NULL;

---

Is it crashing in a success case or a failure case, or in every case?

Quote:

---

Originally Posted by stahta01

My guess is below; note did not read thread.
Tim S.

Code:

---

localMessage = malloc(strlen(errDiff)+1);

---

---

Ooh, good catch... I didn't even notice that errDiff was incremented inside the strLen call... Shouldn't the compiler complain about that?
Anyway, I changed it to:

Code:

---

localMessage = malloc(strlen(errDiff)+1);

---

As well as changing the printf's to fprintf's, but the behavior didn't change one bit. I'll try the standalone program that I can test on Linux with gcc next.

Regards,
Jon Heese

Quote:

---

Originally Posted by rags_to_riches

Can you show how you're calling this (if possible)? It could perhaps be

Code:

---

Message = NULL;

---

, which should probably be

Code:

---

*Message = NULL;

---

---

I cannot show how I'm calling it because I'm not calling it. I'm essentially following an AIX OS API that tells me I need a C module that has that function in it. Whenever 'passwd' is run on this AIX system, the OS then calls that module and function. I don't think the problem is with the inputs to the function, otherwise I would expect the original code (in post #3) to segfault the same way my code does-- and it doesn't do that.

Quote:

---

Is it crashing in a success case or a failure case, or in every case?

---

It only crashes in the case I described in post #1:
If a password meeting the requirements (eg. Passw0rd) is typed in on the first attempt, it crashes.
If a "non-complex" password (eg. password) is typed in on the first attempt, it displays the error message and the user is prompted to retry with a more suitable password. Then, everything works perfectly (good passwords are accepted, bad passwords trigger error message + re-prompt) until the next time a user tries to change his/her password, at which time we start back over at square one.

Regards,
Jon Heese

Okay, I started writing my standalone program based on this code, but I think I'm misunderstanding the way the "Message" variable works... First, here's my code:

Code:

---

#include <ctype.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/errno.h>
#include <ctype.h>
#define errDiff  "Your password must contain at least one character of 3 of the 4 types: lower, upper, digit and special charac$
#define MAX_LEN    64

int main(void)
{
    int rtn = 0, len = 0;
    char uname[MAX_LEN], new[MAX_LEN], old[MAX_LEN], mess[MAX_LEN];
    printf("Enter username: ");
    fgets(uname, sizeof(uname), stdin);
    len = strlen(uname);
    if( uname[len-1] == '\n' )
      uname[len-1] = 0;
    printf("Enter old password: ");
    fgets(old, sizeof(old), stdin);
    len = strlen(old);
    if( old[len-1] == '\n' )
      old[len-1] = 0;
    printf("Enter new password: ");
    fgets(new, sizeof(new), stdin);
    len = strlen(new);
    if( new[len-1] == '\n' )
      new[len-1] = 0;
    rtn = pwdrestrict_method(uname,new,old,mess);
    printf("rtn = %d, UserName=%s, OldPassword=%s, NewPassword=%s, Message=%s\n", rtn, uname, old, new, mess);
    return 0;
}


int pwdrestrict_method (UserName, NewPassword, OldPassword, Message)
char  * UserName;
char  * NewPassword;
char  * OldPassword;
char ** Message;
{
 char * localMessage;
 int    charCnt = 0;
 int    i = 0;
 int    lCnt = 0, uCnt = 0, dCnt = 0, pCnt = 0, cTypes = 0;

 charCnt = strlen(NewPassword);
 for (i=0; i<charCnt; i++) {
  if (islower(NewPassword[i]))
  lCnt++;
  else if (isupper(NewPassword[i]))
  uCnt++;
  else if (isdigit(NewPassword[i]))
  dCnt++;
  else if (ispunct(NewPassword[i]))
  pCnt++;
 }

 fprintf(stderr, "NewPassword=%s lCnt=%d uCnt=%d dCnt=%d pCnt=%d charCnt=%d\n",NewPassword,lCnt,uCnt,dCnt,pCnt,charCnt);
 cTypes = (lCnt!=0) + (uCnt!=0) + (dCnt!=0) + (pCnt!=0);
 fprintf(stderr, "cTypes=%d\n",cTypes);

 if (cTypes < 3) {
  localMessage = malloc(strlen(errDiff)+1);
  strcpy(localMessage, errDiff);
  *Message = localMessage;
  errno = EPERM;
  return 1;
 }

 Message = NULL;
 return 0;
}

---

When I compile and run this code, I don't get a segfault, but I get blank for the debug output for "Message". Being a Java programmer at heart, I am woefully inept at understanding pointer referencing/dereferencing, so I think my deficit here is what's causing my confustion/misunderstanding.

Can someone please give me your thoughts on this code? Any suggestions are very welcome! But go easy on me! :) Thanks.

Regards,
Jon Heese

Quote:

---

Originally Posted by jonheese

Code:

---


 if (cTypes < 3) {
  localMessage = malloc(strlen(errDiff)+1);
  strcpy(localMessage, errDiff);
  *Message = localMessage;
  errno = EPERM;
  return 1;
 }

 Message = NULL;
 return 0;
}

---

---

The "*Message = localMessage;" and "Message = NULL;" are likely to need the same number of stars to work right. I suggest trying one in-front of each. Note: I am not good with using pointer to pointer; but, I do think the both should use the same number of stars.

NOTE: You might just comment out "Message = NULL;" and see if error goes away.

Tim S.

Quote:

---

Originally Posted by stahta01

The "*Message = localMessage;" and "Message = NULL;" are likely to need the same number of stars to work right. I suggest trying one in-front of each. Note: I am not good with using pointer to pointer; but, I do think the both should use the same number of stars.

NOTE: You might just comment out "Message = NULL;" and see if error goes away.

Tim S.

---

Yeah, I tried commenting that line out-- issue still remains. Furthermore, the original code (which does not crash) has the same line in it.

Thanks for your input though!

EDIT: I think you got it!!! When I made the following change (in red), everything appears to be working properly now:

Code:

---

#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/errno.h>
#include <ctype.h>
#define errDiff  "Your password must contain at least one character of 3 of the 4 types: lower, upper, digit and special character\n"

int pwdrestrict_method (UserName, NewPassword, OldPassword, Message)
char  * UserName;
char  * NewPassword;
char  * OldPassword;
char ** Message;
{
 char * localMessage;
 int    charCnt = 0;
 int    i = 0;
 int    lCnt = 0, uCnt = 0, dCnt = 0, pCnt = 0, cTypes = 0;

 charCnt = strlen(NewPassword);
 for (i=0; i<charCnt; i++) {
  if (islower(NewPassword[i]))
  lCnt++;
  else if (isupper(NewPassword[i]))
  uCnt++;
  else if (isdigit(NewPassword[i]))
  dCnt++;
  else if (ispunct(NewPassword[i]))
  pCnt++;
 }

 /*fprintf(stderr, "NewPassword=%s lCnt=%d uCnt=%d dCnt=%d pCnt=%d charCnt=%d\n",NewPassword,lCnt,uCnt,dCnt,pCnt,charCnt);*/
 cTypes = (lCnt!=0) + (uCnt!=0) + (dCnt!=0) + (pCnt!=0);
 /*fprintf(stderr, "cTypes=%d\n",cTypes);*/

 if (cTypes < 3) {
  localMessage = malloc(strlen(errDiff)+1);
  strcpy(localMessage, errDiff);
  *Message = localMessage;
  errno = EPERM;
  return 1;
 }

 /*fprintf(stderr, "before end\n");*/

 *Message = NULL;
 return 0;
}

---

Thank you so much!

Regards,
Jon Heese