Re: [PATCH v3 3/5] staging: rtl8723bs: fix out-of-bounds read in portctrl()

[Luka Gejak]

On Sun Apr 5, 2026 at 12:15 PM CEST, Delene Tchio Romuald wrote:
> In portctrl(), the pointer is advanced by hdrlen + iv_len +
> LLC_HEADER_LENGTH and then 2 bytes are read via memcpy() to extract
> the ether_type field. There is no check that the frame is large
> enough to contain these fields, so a short frame leads to an
> out-of-bounds read on kernel heap memory.
>
> This code is reachable during 802.1X authentication when the station
> is in the ieee8021x_blocked state.
>
> Add a frame length check before the pointer arithmetic and wrap the
> existing ether_type extraction in the else branch so that short
> frames are dropped safely.
>
> Found by reviewing memory operations in the driver.
> Not tested on hardware.
>
> Signed-off-by: Delene Tchio Romuald <delenetchior1@xxxxxxxxx>
> ---
> v3:
>  - Rebased on staging-next
>  - Sent as numbered series with proper Cc from get_maintainer.pl
> v2:
>  - Rebased on staging-next
>
>  drivers/staging/rtl8723bs/core/rtw_recv.c | 28 +++++++++++++++--------
>  1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_recv.c b/drivers/staging/rtl8723bs/core/rtw_recv.c
> index 11ae99e53b86a..8e38cb9791755 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_recv.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_recv.c
> @@ -539,17 +539,25 @@ static union recv_frame *portctrl(struct adapter *adapter, union recv_frame *pre
>  
>  			prtnframe = precv_frame;
>  
> -			/* get ether_type */
> -			ptr = ptr + pfhdr->attrib.hdrlen + pfhdr->attrib.iv_len + LLC_HEADER_LENGTH;
> -			memcpy(&be_tmp, ptr, 2);
> -			ether_type = ntohs(be_tmp);
> -
> -			if (ether_type == eapol_type)
> -				prtnframe = precv_frame;
> -			else {
> -				/* free this frame */
> -				rtw_free_recvframe(precv_frame, &adapter->recvpriv.free_recv_queue);
> +			/* Ensure frame has LLC header and ether_type */
> +			if (pfhdr->len < pattrib->hdrlen +
> +			    pattrib->iv_len + LLC_HEADER_LENGTH + 2) {
> +				rtw_free_recvframe(precv_frame,
> +						   &adapter->recvpriv.free_recv_queue);
>  				prtnframe = NULL;
> +			} else {
> +				/* get ether_type */
> +				ptr += pattrib->hdrlen +
> +				       pattrib->iv_len +
> +				       LLC_HEADER_LENGTH;
> +				memcpy(&be_tmp, ptr, 2);
> +				ether_type = ntohs(be_tmp);
> +
> +				if (ether_type != eapol_type) {
> +					rtw_free_recvframe(precv_frame,
> +							   &adapter->recvpriv.free_recv_queue);
> +					prtnframe = NULL;
> +				}
>  			}
>  		} else {
>  			/* allowed */

LGTM.

Reviewed-by: Luka Gejak <luka.gejak@xxxxxxxxx>

Best regards,
Luka Gejak