iptables: set errno correctly in iptcc_chain_index_alloc
PATCH 1/1 iptables: set errno correctly in iptcc_chain_index_alloc
As reported by Robert Barnhardt, iptcc_chain_index_alloc does not populate errno with the appropriate ENOMEM on allocation failures. This causes incorrect error messages to be passed back to user such as "can't initialize iptables table 'X'" even if the issue was caused by OOM condition. Fix this by passing back ENOMEM if allocation failure occurs.
This closes bugzilla #619.
Phil
libiptc/libiptc.c +2 -1
@@ -502,7 +502,8 @@ static int iptcc_chain_index_alloc(struct xtc_handle *h) h->chain_index =malloc (array_mem);if (h->chain_index ==NULL && array_mem >0 ) { h->chain_index_sz =0 ; -return -ENOMEM ; + errno =ENOMEM ; +return -1 ; }memset (h->chain_index,0 , array_mem); h->chain_index_sz = array_elems;
Florian Westphal
maintainerQuoted 5 lines · Phil Oester
As reported by Robert Barnhardt,
iptcc_chain_index_allocdoes not populate errno with the appropriate ENOMEM on allocation failures. This causes incorrect error messages to be passed back to user such as "can't initialize iptables table 'X'" even if the issue was caused by OOM condition. Fix this by passing back ENOMEM if allocation failure occurs.
Personally I think libraries should not change errno at all.
libiptc/libiptc.c static int iptcc_chain_index_alloc(struct xtc_handle *h) · line 507
Quoted 12 lines
libiptc/libiptc.c +2
-1@@ -502,7 +502,8 @@ static int iptcc_chain_index_alloc(struct xtc_handle *h) h->chain_index =malloc (array_mem);if (h->chain_index ==NULL && array_mem >0 ) { h->chain_index_sz =0 ; -return -ENOMEM ; + errno =ENOMEM ; +return -1 ; }
I don't understand how this changes anything?
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
int main(void) { errno = EINVAL;
void *v = malloc(0xffffffffffffffff);
if (v == 0) perror("malloc"); }Yields "Cannot allocate memory", not "Invalid argument".
Quoted 1 line · Florian Westphal
Personally I think libraries should not change errno at all.
OK, but then we output misleading error messages.
Quoted 1 line
I don't understand how this changes anything?
Simulate an out of memory condition with this patch
@@ -500,9 +500,11 @@ static int iptcc_chain_index_alloc(struct xtc_handle *h)
array_elems, array_mem);
h->chain_index = malloc(array_mem);
- if (h->chain_index == NULL && array_mem > 0) {
+ //if (h->chain_index == NULL && array_mem > 0) {
+ if (1) {
h->chain_index_sz = 0;With the patch, the error message returned to user:
...can't initialize iptables table `filter': Memory allocation problem
without the patch:
...can't initialize iptables table `filter': Incompatible with this kernel
The former seems better, no?
Phil
Florian Westphal
maintainerQuoted 12 lines · Phil Oester
Simulate an out of memory condition with this patch
(quoted diff) +2
-1@@ -500,9 +500,11 @@ static int iptcc_chain_index_alloc(struct xtc_handle *h) array_elems, array_mem); h->chain_index = malloc(array_mem); - if (h->chain_index == NULL && array_mem > 0) { + //if (h->chain_index == NULL && array_mem > 0) { + if (1) { h->chain_index_sz = 0;With the patch, the error message returned to user:
Quoted 7 lines
...can't initialize iptables table `filter': Memory allocation problem
without the patch:
...can't initialize iptables table `filter': Incompatible with this kernel
The former seems better, no?
Yes, but malloc didn't fail, so malloc didn't set errno.
My point is, that we should not muck with errno, especially after libc functions that usually already set it on error.
Quoted 2 lines · Florian Westphal
My point is, that we should not muck with errno, especially after libc functions that usually already set it on error.
# grep -c 'errno = ' libiptc/libiptc.c 52
But ok, we can avoid adding yet another instance and drop this patch.
Phil