[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Freeipmi-devel] Re: [llnl-devel] checking chksums, possible framework c
[Freeipmi-devel] Re: [llnl-devel] checking chksums, possible framework change needed??
Tue, 18 Nov 2003 16:22:56 -0800
I always kept changing the order of chksum and comp_code
validation. And I decided to postpone it until disaster strikes again.
Let us fix it now.
Validity of the comp_code itself depends on the chksum. But location
of chksum depends on comp_code. There seems to be a cyclic dependency.
I think it is OK, not to validate the chksum if comp_code is not
success. In that case we can check comp_code first.
We can double-check the chksum and then validate comp_code
if (pkt.chksum != ipmi_chksum((u_int8_t *) &pkt.msg.rs,
(sizeof (pkt.msg.rs) + sizeof(pkt.data))))
memcpy (&err_rs, &pkt.msg.rs, sizeof (ipmi_err_rs_t));
if (err_rs.chksum != ipmi_chksum((u_int8_t *) &err_rs.msg.rs,
(sizeof (err_rs.msg.rs) + sizeof(err_rs.data))))
REALLY BAD CHKSUM
AND THEN VALIDATE COMP_CODE
typedef struct ipmi_err_rs
// just for writing generic chksum macros
There is also one more approach - using "c union", but too may nesting of
structures aren't elegant.
Albert Chu <address@hidden> writes:
>In your ipmi-wrapper.c code, you do checksum checks like the following:
>if (pkt.chksum != ipmi_chksum((u_int8_t *) &pkt.msg.rs,
> (sizeof (pkt.msg.rs) + sizeof(pkt.data))))
> // yell and scream about checksum errors
>However this is incorrect. The reason is that on a completion code
>error (for example, a bad username or bad password error), response data
>may not be returned. Thus, the packet received will be smaller than
>expected. Thus, the response packet's checksum field may not be copied
>into the pkt.chksum field properly. And the above chksum check bombs.
>For example, a get_session_challenge_response is 42 bytes (rmcp +
>session + rs_msg + data + chksum). However, if you passed a bad
>username during the get_session_challenge_request command, the response
>will only be 23 bytes (the only response data is the completion code, no
>session_id or challenge string is returned). Therefore, calling
>ipmi_chksum using your above example will fail, when in fact, it
>shouldn't fail. The checksums are probably fine, its the completion
>code that is the problem.
>I think the correct way is to do this is as follows:
>len = ipmi_recvfrom(fd, &rmcp_hdr, &ipmi_pkt, sizeof(ipmi_pkt));
>if (len <= 0)
> // blah blah
>if (ipmi_response_chksum(&ipmi_pkt.msg.rs, len) == 0)
> // blah blah yell and scream
>I think the ipmi_recvfrom should return the length of data stored
>in the ipmi_pkt buffer. Then, the ipmi_chksum_response function
>assumes the last byte of the buffer passed in is the checksum field.
>Let me know what you think ...
>Lawrence Livermore National Laboratories
>llnl-devel mailing list
Free as in Freedom <www.gnu.org>