• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 533
  • Last Modified:

Getting segmentation fault while trying to insert two iptables rules using the same handle

By using the following code i'm getting a segmentation fault:
IPfilter filterobject;
b=0;
while(b<10)
{
filterobject.insert_rule((char *)"23.3.223.1");
b++;
}

If i rerun the constructor every loop by recreating a new object on every loop it works fine.
But the creation of a new handle takes a long time if the iptables table contains about 20000 entries.

By runing ddd it displays that the segmentation fault occours in the iptcc_find_label() method wich is in included in any linked library. I inserted some debug output on several positions into my code and found out that my handle become NULL after calling the "iptc_insert_entry" function.

Do i need to recreate a new handle on every modification?
Is there any error on my code?

Thanks.
class IPfilter{
private:
  iptc_handle_t h;
  char *tablename;
  const struct ipt_entry *e;
  struct ipt_entry_target * append_target(struct ipt_entry *entry, const char *name, size_t target_data_size);
public:
  IPfilter();
  bool insert_rule(char *dstip);
};
 
IPfilter::IPfilter()
{
h=NULL;
e=NULL;
tablename=(char *)malloc(7);
sprintf(tablename,"mangle");
this->h=iptc_init(tablename);
if (!(this->h))
printf("Error createing Iptables Structure!");
};
 
bool IPfilter::insert_rule(char *dstip)
{
       int result;
       int unused = 0;
       struct ipt_entry *new_entry;
       struct ipt_entry_target *target;
 
       if (!(new_entry =(ipt_entry*) calloc(1, sizeof(struct ipt_entry))))
       {
         perror("calloc");
         return false;
       }
       inet_aton(dstip, &new_entry->ip.dst);
       inet_aton("255.255.255.255", &new_entry->ip.dmsk);
       sprintf(new_entry->ip.outiface, "eth0");
       new_entry->ip.proto = IPPROTO_TCP;
 
       // initialize entry parameters
       new_entry->target_offset = sizeof(struct ipt_entry);
       new_entry->next_offset = sizeof(struct ipt_entry);
       target = this->append_target(new_entry, "DROP", sizeof(int));
       memcpy(&target->data[0], &unused, sizeof(int));
       result =  iptc_insert_entry("PREROUTING", new_entry,0, &(this->h));
       if (!result)
         printf("%s\n", iptc_strerror(errno));
       result = iptc_commit( &(this->h));
       if (!result)
       printf("%s\n", iptc_strerror(errno));
       free (new_entry);
       return true;
};
 
struct ipt_entry_target * IPfilter::append_target(struct ipt_entry *entry, const char *name, size_t target_data_size)
{
        struct ipt_entry_target *target;
        size_t target_size = 0;
 
        target_size += IPT_ALIGN(sizeof(struct ipt_entry_target));
        target_size += IPT_ALIGN(target_data_size);
        if (!(entry =(ipt_entry*) realloc(entry, entry->next_offset + target_size)))
        {
                perror("realloc");
                return NULL;
        }
        target = (struct ipt_entry_target *)((int)entry + entry->next_offset);
        entry->next_offset += target_size;
        memset(target, 0, target_size);
        strncpy(target->u.user.name, name, IPT_FUNCTION_MAXNAMELEN);
        target->u.target_size = target_size;
        return target;
};

Open in new window

0
ktd85
Asked:
ktd85
  • 10
  • 10
1 Solution
 
Infinity08Commented:
There's a small problem with this line :

>>         if (!(entry =(ipt_entry*) realloc(entry, entry->next_offset + target_size)))

It works on a local copy of the struct ipt_entry*, so after calling append_target, the insert_rule method will still be using its own copy of the struct ipt_entry*, which might or might not be the same as what realloc returned.


So, you'll have to make sure that the value returned by realloc makes it back to the insert_rule method.
0
 
ktd85Author Commented:
Would it be corrected by calling it like this:
target = this->append_target(&new_entry, "DROP", sizeof(int));

Do you think that can cause the segmentation faultĀ“if i call the function more the one time from a freshly created object ?
0
 
Infinity08Commented:
>> target = this->append_target(&new_entry, "DROP", sizeof(int));

If you also change the append_target method accordingly :

        struct ipt_entry_target * IPfilter::append_target(struct ipt_entry **pentry, const char *name, size_t target_data_size)
        {
                /* rest of the code as it was, but using *pentry instead of entry */
        }


>> Do you think that can cause the segmentation faultĀ“if i call the function more the one time from a freshly created object ?

It could. If realloc places the memory block elsewhere, then the new_entry pointer in the insert_rule method no longer points to allocated memory, and could have been overwritten by something else.
It's this overwriting that could cause a segmentation fault (because pointers, and offset fields are overwritten too).
0
Veeam and MySQL: How to Perform Backup & Recovery

MySQL and the MariaDB variant are among the most used databases in Linux environments, and many critical applications support their data on them. Watch this recorded webinar to find out how Veeam Backup & Replication allows you to get consistent backups of MySQL databases.

 
ktd85Author Commented:
If i call the function only once there is no problem. If i recreate the object and then call the function there will be no problem too.

If i create a object of the class IPfilter and call the function insert_rule twice I'm getting a segmentation fault.
0
 
ktd85Author Commented:
Sorry, ignore my poste above.
I corrected the mentioned issues and now get a compiler error:
src/ipfilter.c:82: error: extra qualification IPfilter:: on member append_target
src/ipfilter.c: In member function xt_entry_target* IPfilter::append_target(ipt_entry**, const char*, size_t):
src/ipfilter.c:182: error: request for member next_offset in * pentry, which is of non-class type ipt_entry*
src/ipfilter.c:187: error: request for member next_offset in * pentry, which is of non-class type ipt_entry*
src/ipfilter.c:188: error: request for member next_offset in * pentry, which is of non-class type ipt_entry*
make: *** [src/ipfilter.o] Error 1


Now code of method append_target attached.
  struct ipt_entry_target * IPfilter::append_target(struct ipt_entry **pentry, const char *name, size_t target_data_size)
{
        struct ipt_entry_target *target;
        size_t target_size = 0;
 
        target_size += IPT_ALIGN(sizeof(struct ipt_entry_target));
        target_size += IPT_ALIGN(target_data_size);
        if (!(pentry =(ipt_entry*) realloc(pentry, pentry->next_offset + target_size)))
        {
                perror("realloc");
                return NULL;
        }
        target = (struct ipt_entry_target *)((int)pentry + pentry->next_offset);
        pentry->next_offset += target_size;
        memset(target, 0, target_size);
        strncpy(target->u.user.name, name, IPT_FUNCTION_MAXNAMELEN);
        target->u.target_size = target_size;
        return target;
};

Open in new window

0
 
Infinity08Commented:
You should dereference pentry before using it, ie. *pentry instead of just pentry : (I also modified a few other things and commented them)
struct ipt_entry_target * IPfilter::append_target(struct ipt_entry **pentry, const char *name, size_t target_data_size)
{
        struct ipt_entry *tmp_ptr = 0;
        struct ipt_entry_target *target = 0;       // <--- initialize pointer
        size_t target_size = 0;
 
        target_size += IPT_ALIGN(sizeof(struct ipt_entry_target));
        target_size += IPT_ALIGN(target_data_size);
        if (!(tmp_ptr =(ipt_entry*) realloc(*pentry, (*pentry)->next_offset + target_size)))       // <--- two-step realloc to avoid memory leaks in case realloc fails
        {
                perror("realloc");
                return NULL;         // <--- make sure you check for NULL in insert_rule after calling append_target
        }
        *pentry = tmp_ptr;       // <--- two-step realloc to avoid memory leaks in case realloc fails
        target = (struct ipt_entry_target *)((unsigned char*)*pentry + (*pentry)->next_offset);    // <--- casting to a 1 byte pointer type (unsigned char*) instead of int
        (*pentry)->next_offset += target_size;
        memset(target, 0, target_size);
        strncpy(target->u.user.name, name, IPT_FUNCTION_MAXNAMELEN);
        target->u.target_size = target_size;
        return target;
}                           // <--- notice that I removed the ; here - it's not necessary

Open in new window

0
 
Infinity08Commented:
You'll of course also have to modify the method prototype with the new struct ipt_entry **pentry.
0
 
ktd85Author Commented:
I corrected all the items you metioned but I'm still receiving the segmentation fault.
Current code attached.

Do you have any other idea?
class IPfilter{
private:
  iptc_handle_t h;
  char *tablename;
  const struct ipt_entry *e;
 
  void print_ip(char *prefix, u_int32_t ip, u_int32_t mask, int invert);
  void print_rule(const struct ipt_entry *e, iptc_handle_t *h, const char *chain, int counters);
  struct ipt_entry_target * append_target(struct ipt_entry **pentry, const char *name, size_t target_data_size);
public:
  IPfilter();
  bool insert_rule(char *dstip);
};
 
IPfilter::IPfilter()
{
h=NULL;
e=NULL;
tablename=(char *)malloc(7);
sprintf(tablename,"mangle");
 
this->h=iptc_init(tablename);
if (!(this->h))
printf("Error createing Iptables Structure!");
}
 
bool IPfilter::insert_rule(char *dstip)
{
       int result;
       int unused = 0;
       struct ipt_entry *new_entry;
       struct ipt_entry_target *target;
 
       if (!(new_entry =(ipt_entry*) calloc(1, sizeof(struct ipt_entry))))
       {
         perror("calloc");
         return false;
       }
       inet_aton(dstip, &new_entry->ip.dst);
       inet_aton("255.255.255.255", &new_entry->ip.dmsk);
       sprintf(new_entry->ip.outiface, "eth0");
       new_entry->ip.proto = 6;
 
       // initialize entry parameters
       new_entry->target_offset = sizeof(struct ipt_entry);
       new_entry->next_offset = sizeof(struct ipt_entry);
       target = this->append_target(&new_entry, "DROP", sizeof(int));
       if (target==NULL) return false;
       memcpy(&target->data[0], &unused, sizeof(int));
       result =  iptc_append_entry("PREROUTING", new_entry, &(this->h));
       if (!result)
         printf("%s\n", iptc_strerror(errno));
       result = iptc_commit( &(this->h));
       if (!result)
       printf("%s\n", iptc_strerror(errno));
       free (new_entry);
       return true;
}
 
struct ipt_entry_target * IPfilter::append_target(struct ipt_entry **pentry, const char *name, size_t target_data_size)
{
        struct ipt_entry *tmp_ptr = 0;
        struct ipt_entry_target *target = 0;       // <--- initialize pointer
        size_t target_size = 0;
        target_size += IPT_ALIGN(sizeof(struct ipt_entry_target));
        target_size += IPT_ALIGN(target_data_size);
        if (!(tmp_ptr =(ipt_entry*) realloc(*pentry, (*pentry)->next_offset + target_size)))       // <--- two-step realloc to avoid memory leaks in case realloc fails
        {
        perror("realloc");
        return NULL;         // <--- make sure you check for NULL in insert_rule after calling append_target
        }
        *pentry = tmp_ptr;       // <--- two-step realloc to avoid memory leaks in case realloc fails
        target = (struct ipt_entry_target *)((unsigned char*)*pentry + (*pentry)->next_offset);    // <--- casting to a 1 byte pointer type (unsigned char*) instead of int
        (*pentry)->next_offset += target_size;
        memset(target, 0, target_size);
        strncpy(target->u.user.name, name, IPT_FUNCTION_MAXNAMELEN);
        target->u.target_size = target_size;
        return target;
}

Open in new window

0
 
Infinity08Commented:
>> but I'm still receiving the segmentation fault.

In the same location ?
0
 
ktd85Author Commented:
Yes.
Calling once in a instance of the Classe no problem. Calling twice in one instance causes segmentation fault. Found out, that the Handle h, which is initialized by the contructor becomes NULL after the first run.
0
 
Infinity08Commented:
>> Found out, that the Handle h, which is initialized by the contructor becomes NULL after the first run.

That would explain the segmentation fault.

Now, we just have to find out why heh.


I can't find anything in the code you posted that would do that ... What happens to the IPFilter object between the two calls ?
0
 
ktd85Author Commented:
What do you mean by "between two calls"?
The While-Loop while goe through and repeat from top.
0
 
Infinity08Commented:
>> The While-Loop while goe through and repeat from top.

So, it's just the while loop you posted in your first post - nothing more ? That's the exact code ?
0
 
ktd85Author Commented:
Yes, thats the original code i used to test the function's performance.
0
 
Infinity08Commented:
Can you find out at what point h is set to 0 ? You can use a debugger for that (just set a watch on h).
0
 
ktd85Author Commented:
My Installation of ddd complains that <director fo my project>\<<c++ namspaces>> ist not found.
0
 
Infinity08Commented:
Can you try adding these lines then and see what output it gives :

       // initialize entry parameters
       new_entry->target_offset = sizeof(struct ipt_entry);
       new_entry->next_offset = sizeof(struct ipt_entry);
       if (!(this->h)) printf(stderr, "this->h == NULL (A)\n");             // <--- add
       else printf(stderr, "this->h != NULL (A)\n");                        // <--- add
       target = this->append_target(&new_entry, "DROP", sizeof(int));
       if (!(this->h)) printf(stderr, "this->h == NULL (B)\n");             // <--- add
       else printf(stderr, "this->h != NULL (B)\n");                        // <--- add
       if (target==NULL) return false;
       memcpy(&target->data[0], &unused, sizeof(int));
       result =  iptc_append_entry("PREROUTING", new_entry, &(this->h));
       if (!(this->h)) printf(stderr, "this->h == NULL (C)\n");             // <--- add
       else printf(stderr, "this->h != NULL (C)\n");                        // <--- add
       if (!result)
         printf("%s\n", iptc_strerror(errno));
       result = iptc_commit( &(this->h));
       if (!(this->h)) printf(stderr, "this->h == NULL (D)\n");             // <--- add
       else printf(stderr, "this->h != NULL (D)\n");                        // <--- add
       if (!result)
       printf("%s\n", iptc_strerror(errno));

Open in new window

0
 
ktd85Author Commented:
The Output looks like this:
this->h != NULL (A)
this->h != NULL (B)
this->h != NULL (C)
this->h == NULL (D)
this->h == NULL (A)
this->h == NULL (B)
Segmentation fault
0
 
Infinity08Commented:
>> this->h == NULL (D)

It seems that the iptc_commit sets it to NULL. Is that normal behavior ?

What if you call it after the while loop (so not in the insert_rule) ?
0
 
ktd85Author Commented:
Thank You VERY VERY much.  My Problem is solved!!!!

Thanks.
0

Featured Post

Technology Partners: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

  • 10
  • 10
Tackle projects and never again get stuck behind a technical roadblock.
Join Now