staging: fix crashes when some external APIs fail #186
Conversation
When bind fails, print error messages and return.
When fopen fails, print error message and return.
When the external APIs fail, print error message.
There was a problem hiding this comment.
Please add a Signed-off-by line to your commit message, see the file SubmittingPatches for details.
Please also prefix the subject of the commit (i.e. the first line) with "mausezahn:".
There was a problem hiding this comment.
Please add a Signed-off-by line to your commit message, see the file SubmittingPatches for details.
Please also prefix the subject of the commit (i.e. the first line) with "mausezahn:".
staging/lookupdev.c
Outdated
| @@ -311,7 +311,11 @@ int get_dev_params (char *name) | |||
| psock.sll_hatype = 0; // unsigned short - Header type //ARPHRD_ETHER | |||
| psock.sll_pkttype = 0; // unsigned char - Packet type | |||
| psock.sll_halen = 6; // unsigned char - Length of address | |||
| bind(ps, (const struct sockaddr *) &psock, sizeof(psock)); // <= !!! | |||
| if (bind(ps, (const struct sockaddr *) &psock, sizeof(psock))==-1) { // <= !!! | |||
There was a problem hiding this comment.
Please add spaces around " == " and get rid of the trailing comment.
staging/lookupdev.c
Outdated
| @@ -228,7 +228,11 @@ int get_dev_params (char *name) | |||
|
|
|||
| // 2. find network, gateway, and mask | |||
|
|
|||
| fd = fopen("/proc/net/route", "r"); | |||
| fd = fopen("/proc/net/route", "r"); | |||
| if (fd==NULL) { | |||
There was a problem hiding this comment.
Please add a Signed-off-by line to your commit message, see the file SubmittingPatches for details.
Please also prefix the subject of the commit (i.e. the first line) with "mausezahn:".
staging/cli.c
Outdated
| @@ -525,6 +525,9 @@ int cli(void) | |||
|
|
|||
| // Create a socket | |||
| s = socket(AF_INET, SOCK_STREAM, 0); | |||
| if (s==-1) { | |||
staging/cli.c
Outdated
| @@ -525,6 +525,9 @@ int cli(void) | |||
|
|
|||
| // Create a socket | |||
| s = socket(AF_INET, SOCK_STREAM, 0); | |||
| if (s==-1) { | |||
| perror("socket"); | |||
There was a problem hiding this comment.
I think we should return here instead of keep on working on the invalid socket s further down.
staging/cli.c
Outdated
| @@ -525,6 +525,9 @@ int cli(void) | |||
|
|
|||
| // Create a socket | |||
| s = socket(AF_INET, SOCK_STREAM, 0); | |||
| if (s==-1) { | |||
| perror("socket"); | |||
| } | |||
| setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)); | |||
There was a problem hiding this comment.
setsockopt can also fail, so it's return value should be checked as well.
staging/cli.c
Outdated
| @@ -541,14 +544,18 @@ int cli(void) | |||
| servaddr.sin_family = AF_INET; | |||
| inet_aton(mz_listen_addr, &servaddr.sin_addr); | |||
| servaddr.sin_port = htons(mz_port); | |||
| bind(s, (struct sockaddr *)&servaddr, sizeof(servaddr)); | |||
| if (bind(s, (struct sockaddr *)&servaddr, sizeof(servaddr))==-1) { | |||
staging/cli.c
Outdated
| @@ -541,14 +544,18 @@ int cli(void) | |||
| servaddr.sin_family = AF_INET; | |||
| inet_aton(mz_listen_addr, &servaddr.sin_addr); | |||
| servaddr.sin_port = htons(mz_port); | |||
| bind(s, (struct sockaddr *)&servaddr, sizeof(servaddr)); | |||
| if (bind(s, (struct sockaddr *)&servaddr, sizeof(servaddr))==-1) { | |||
| perror("bind"); | |||
There was a problem hiding this comment.
I think we should return here in case of an error.
staging/cli.c
Outdated
|
|
||
| if (!quiet) { | ||
| fprintf(stderr, "Mausezahn accepts incoming Telnet connections on %s:%i.\n", mz_listen_addr, mz_port); | ||
| } | ||
|
|
||
| // Wait for a connection | ||
| listen(s, 50); | ||
| if (listen(s, 50)==-1) { |
staging/cli.c
Outdated
|
|
||
| if (!quiet) { | ||
| fprintf(stderr, "Mausezahn accepts incoming Telnet connections on %s:%i.\n", mz_listen_addr, mz_port); | ||
| } | ||
|
|
||
| // Wait for a connection | ||
| listen(s, 50); | ||
| if (listen(s, 50)==-1) { | ||
| perror("listen"); |
There was a problem hiding this comment.
I think we should return here in case of an error.
When the external APIs fail, print error message. Signed-off-by: Zhouyang Jia [email protected]
When the APIs fail, check their return value and return. Signed-off-by: Zhouyang Jia [email protected]
|
Your original commits (lacking the Signed-off-by lines) still appear in the PR. Please squash them with the following commits. I think they can all actually be in one single commit as it all deals with the same issue. |
staging/cli.c
Outdated
| @@ -525,10 +525,12 @@ int cli(void) | |||
|
|
|||
| // Create a socket | |||
| s = socket(AF_INET, SOCK_STREAM, 0); | |||
| if (s==-1) { | |||
| perror("socket"); | |||
There was a problem hiding this comment.
The perror call can stay here (also in the changes below). Please also check the return value of the cli function in the caller:
netsniff-ng/staging/mausezahn.c
Line 805 in 1e1c5f9
You could also add an error message like "failed to create socket for mausezahn CLI" and return an accordingly (which is then again handled in the caller.
Some potential API bugs may cause crashes, which are mainly caused by insufficient error handling.
This patch fixes these reported potential bugs by adding error handling code.
Signed-off-by: Zhouyang Jia [email protected]