linux-kernel - Re: [PATCH 1/1] Prevent the ipmi_bt_sm driver from attempting to send larger messages than hardware accept

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5463771C.1070603@acm.org>
Date:	Wed, 12 Nov 2014 09:05:00 -0600
From:	Corey Minyard <minyard@....org>
To:	Dmitry Rakhchev <rda@...eonpoint.com>
CC:	openipmi-developer@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org, Rocky Craig <rocky.craig@...com>
Subject: Re: [PATCH 1/1] Prevent the ipmi_bt_sm driver from attempting to
 send larger messages than hardware accept

This is probably a good idea, but I would expect that the BMC would
respond with a "Request data field length limit exceeded. (C8h)"
completion code in this case instead of being silent.  Though as far as
I can tell there's nothing in the spec that says what should happen in
this case.

Rocky, what do you think?

-corey

On 11/11/2014 04:40 AM, Dmitry Rakhchev wrote:
> While testing our IPMC implementation for maximal message size I've
> encountered hardware buffer overflow in BT driver.
>
> The problem is visible as a timeout on the driver side, and our IPMC received
> garbage instead of message.
>
> To reproduce:
> # ipmitool raw 0xa 0x12 0x0 0x0 0x0   0x01 0x00 0x00 0x01 0x08 0x10 0x00 0xe6 0x01 0x07 0x19 0x00 0x26 0x70 0xc3 0x50 0x50 0x53 0xd0 0x42 0x4d 0x52 0x2d 0x41 0x32 0x46 0x2d 0x41 0x54 0x43 0x41 0x2d 0x42 0x54 0x52 0xca 0x50 0x50 0x53 0x78 0x78 0x78 0x78 0x78 0x78 0x78 0xc2 0x41 0x20 0xcc 0x66 0x72 0x75 0x2d 0x69 0x6e 0x66 0x01
> Unable to send RAW command (channel=0x0 netfn=0xa lun=0x0 cmd=0x12 rsp=0xc3): 
> Timeout
>
> From dmesg:
> [  353.844011] IPMI BT: timeout in RD_WAIT [ ] 1 retries left
> [  355.845011] IPMI BT: timeout in RD_WAIT [ ] 
> [  355.845017] failed 2 retries, sending error response
>
> Dump of messages on IPMC:
> <B>: <-- { 18 35 01 }
> <B>: --> { 1C 35 01 00 12 80 01 20 02 2D 0A 40 00 DA BB }
> <B>: <-- { B0 36 00 00 }
> <B>: --> { B4 36 00 00 00 32 01 00 }
> <B>: <-- { B0 37 01 00 }
> <B>: --> { B4 37 01 00 00 42 84 FF 00 00 00 }
> <B>: <-- { 28 }
> <B>: <-- { 28 }
>
> Last two messages are the corrupted raw message from ipmitool above (extra 0x01 byte overwrite the length).
>
> Maximum message size check in bt_start_transaction shall respect maximum 
> request size reported by Get BT Capabilities.
>
> Solution: store the size returned from IPMC and use it to check maximal 
> allowed message size. Use minimal value 63 required by IPMI until the real
> maximum is known.
>
> Changes tested on 3.15.10-200.fc20.i686+PAE.
>
> Signed-off-by: Dmitry Rakhchev <rda@...eonpoint.com>
> ---
>  drivers/char/ipmi/ipmi_bt_sm.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_bt_sm.c b/drivers/char/ipmi/ipmi_bt_sm.c
> index 61e7161..c64ad02 100644
> --- a/drivers/char/ipmi/ipmi_bt_sm.c
> +++ b/drivers/char/ipmi/ipmi_bt_sm.c
> @@ -58,6 +58,7 @@ MODULE_PARM_DESC(bt_debug, "debug bitmask, 1=enable, 2=messages, 4=states");
>  #define BT_NORMAL_TIMEOUT	5	/* seconds */
>  #define BT_NORMAL_RETRY_LIMIT	2
>  #define BT_RESET_DELAY		6	/* seconds after warm reset */
> +#define BT_MINIMAL_MESSAGE_SIZE	63	/* Does not include the length byte */
>  
>  /*
>   * States are written in chronological order and usually cover
> @@ -105,6 +106,7 @@ struct si_sm_data {
>  	int		nonzero_status;	/* hung BMCs stay all 0 */
>  	enum bt_states	complete;	/* to divert the state machine */
>  	int		BT_CAP_outreqs;
> +	int		BT_CAP_inbufsz;
>  	long		BT_CAP_req2rsp;
>  	int		BT_CAP_retries;	/* Recommended retries */
>  };
> @@ -203,6 +205,7 @@ static unsigned int bt_init_data(struct si_sm_data *bt, struct si_sm_io *io)
>  	bt->complete = BT_STATE_IDLE;	/* end here */
>  	bt->BT_CAP_req2rsp = BT_NORMAL_TIMEOUT * USEC_PER_SEC;
>  	bt->BT_CAP_retries = BT_NORMAL_RETRY_LIMIT;
> +	bt->BT_CAP_inbufsz = BT_MINIMAL_MESSAGE_SIZE;
>  	/* BT_CAP_outreqs == zero is a flag to read BT Capabilities */
>  	return 3; /* We claim 3 bytes of space; ought to check SPMI table */
>  }
> @@ -229,7 +232,7 @@ static int bt_start_transaction(struct si_sm_data *bt,
>  
>  	if (size < 2)
>  		return IPMI_REQ_LEN_INVALID_ERR;
> -	if (size > IPMI_MAX_MSG_LENGTH)
> +	if ((size + 1) > bt->BT_CAP_inbufsz)
>  		return IPMI_REQ_LEN_EXCEEDED_ERR;
>  
>  	if (bt->state == BT_STATE_LONG_BUSY)
> @@ -651,6 +654,7 @@ static enum si_sm_result bt_event(struct si_sm_data *bt, long time)
>  		bt_init_data(bt, bt->io);
>  		if ((i == 8) && !BT_CAP[2]) {
>  			bt->BT_CAP_outreqs = BT_CAP[3];
> +			bt->BT_CAP_inbufsz = BT_CAP[4];
>  			bt->BT_CAP_req2rsp = BT_CAP[6] * USEC_PER_SEC;
>  			bt->BT_CAP_retries = BT_CAP[7];
>  		} else

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/