#319 Interface safe re-open.

[Dmitry Bazhenov]

Currently, interface-management code in the ipmitool does not allow safe interface re-opening (i.e. closing and opening again). It is because the session is allocated in the interface setup callback while is freed in the close callback. So, normal re-opening of the interface, which can be required for example durng the HPM.1 upgrade, leads to segmentation fault. That's why in the ipmi_hpmfwupg.c instead of normal closing interface, directly access the interface data for subsequent re-opening.
The proposed patch splits the session data into parameters part, which is setup once per ipmitool lauch, and the actual session part, which is allocated and initialized during open and freed/uninitialized during closing of the interface. This make it safe to close and open interface several times.

[Zdenek Styblik]

Ticket moved from /p/ipmitool/patches/95/

[Dmitry Bazhenov]

Hello, Zdenek,

Do you want me to rebase the patch against the latest trunk?

Regards,
Dmitry

[Dmitry Bazhenov]

Rebased against the top of branch.

[Zdenek Styblik]

I got bunch of .rej :\

[Zdenek Styblik]

I wish this patch wasn't so big :(

[Dmitry Bazhenov]

Rebased against the top of trunk. Removed irrelevant changes (will post them as separate bug-fixes).

[Zdenek Styblik]

Patch introduces three new issues pointed out by Coverity(attached). Also:

/* ??? What's the point of having this around if it's commented out? */
+   //session->v2_data.console_id       = 0x00;
+   //session->v2_data.bmc_id           = 0x00;
+   //memset(session->v2_data.sik, 0, IPMI_SIK_BUFFER_SIZE);

And don't put #define in the middle of struct declaration.

[Dmitry Bazhenov]

Addressed review comments.

[Zdenek Styblik]

lan.c ~ 2071 - use of uninitialized addr as a source in memcpy() is still there. Can I do the following to fix it?

memset(&add, 0, sizeof(addr));

[Dmitry Bazhenov]

Hello, Zdenek,

While this ->addr field is mostly useless, it is not correctly initialized. Thanks for noticing.
The best way to resolve the issue is to replace the memcpy with the following code:
- memcpy(&s->addr, &addr, sizeof (s->addr));
+ s->addrlen = sizeof(s->addr);
+ getsocketname(intf->fd, &s->addr, &s->addrlen);

This will fill the addr and addrlen fields with the actual socket name disregarding whether it IPv4 or IPv6.

Regards,
Dmitry

[Zdenek Styblik]

Committed into Git. Thanks.