diff mbox series

[07/10] libsframe: guard against buffer over-read in sframe_fde_tbl_init

Message ID 20260827224146.3391610-8-ibhagatgnu@gmail.com
State New
Headers
Series fix two PRs and guard against bad data |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Indu Bhagat Aug. 27, 2026, 10:41 p.m. UTC 
  In SFrame V3, the FDE attribute data is stored near the SFrame FREs.  In
sframe_fde_tbl_init(), for SFrame V3 format,
fdep->sfdi_func_start_fre_off was used to index into fre_buf without
verifying if sfdi_func_start_fre_off + sizeof(sframe_func_desc_attr_v3)
<= fre_buf_len.  If left unchecked, this may cause buffer over-reads in case
of invalid SFrame data.

Pass FRE buffer len to sframe_fde_tbl_init(), and validate
sfdi_func_start_fre_off against that guard before reading FDE attribute
data.
---
 libsframe/sframe.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Jens Remus Sept. 7, 2026, 12:44 p.m. UTC | #1 
On 8/28/2026 12:41 AM, Indu Bhagat wrote:
> In SFrame V3, the FDE attribute data is stored near the SFrame FREs.  In
> sframe_fde_tbl_init(), for SFrame V3 format,
> fdep->sfdi_func_start_fre_off was used to index into fre_buf without
> verifying if sfdi_func_start_fre_off + sizeof(sframe_func_desc_attr_v3)
> <= fre_buf_len.  If left unchecked, this may cause buffer over-reads in case
> of invalid SFrame data.
> 
> Pass FRE buffer len to sframe_fde_tbl_init(), and validate
> sfdi_func_start_fre_off against that guard before reading FDE attribute
> data.
> ---
>  libsframe/sframe.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

Reviewed-by: Jens Remus <jremus@linux.ibm.com>

> diff --git a/libsframe/sframe.c b/libsframe/sframe.c

> @@ -140,8 +140,8 @@ sframe_fde_tbl_alloc (sf_fde_tbl **fde_tbl, unsigned int num_fdes)
>  
>  static int
>  sframe_fde_tbl_init (sf_fde_tbl *fde_tbl, const char *fde_buf,
> -		     const char *fre_buf, size_t *fidx_size,
> -		     unsigned int num_fdes, uint8_t ver)
> +		     const char *fre_buf, size_t fre_buf_len,
> +		     size_t *fidx_size, unsigned int num_fdes, uint8_t ver)
>  {
>    if (ver == SFRAME_VERSION_3 && SFRAME_VERSION == SFRAME_VERSION_3)
>      {
> @@ -155,6 +155,10 @@ sframe_fde_tbl_init (sf_fde_tbl *fde_tbl, const char *fde_buf,
>  	  fde_tbl->entry[i].func_start_fre_off = fdep->sfdi_func_start_fre_off;
>  	  /* V3 organizes the following data closer to the SFrame FREs for the
>  	     function.  Access them via the sfde_func_start_fre_off.  */
> +	  if (fdep->sfdi_func_start_fre_off > fre_buf_len
> +	      || (sizeof (sframe_func_desc_attr_v3)
> +		  > (fre_buf_len - fdep->sfdi_func_start_fre_off)))
> +	    return SFRAME_ERR;
>  	  const sframe_func_desc_attr_v3 *fattr
>  	    = (sframe_func_desc_attr_v3 *)(fre_buf
>  					   + fdep->sfdi_func_start_fre_off);
> @@ -1535,7 +1539,7 @@ sframe_decode (const char *sf_buf, size_t sf_size, int *errp)
>  
>    /* SFrame FDEs are at an offset of sfh_fdeoff from SFrame header end.  */
>    if (sframe_fde_tbl_init (dctx->sfd_funcdesc, frame_buf + dhp->sfh_fdeoff,
> -			   frame_buf + dhp->sfh_freoff,
> +			   frame_buf + dhp->sfh_freoff, dhp->sfh_fre_len,
>  			   &fidx_size, dhp->sfh_num_fdes, sfp->sfp_version))
>      {
>        sframe_ret_set_errno (errp, SFRAME_ERR_BUF_INVAL);

Regards,
Jens
diff mbox series

Patch

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index 288d7c0d873..f047342a782 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -140,8 +140,8 @@  sframe_fde_tbl_alloc (sf_fde_tbl **fde_tbl, unsigned int num_fdes)
 
 static int
 sframe_fde_tbl_init (sf_fde_tbl *fde_tbl, const char *fde_buf,
-		     const char *fre_buf, size_t *fidx_size,
-		     unsigned int num_fdes, uint8_t ver)
+		     const char *fre_buf, size_t fre_buf_len,
+		     size_t *fidx_size, unsigned int num_fdes, uint8_t ver)
 {
   if (ver == SFRAME_VERSION_3 && SFRAME_VERSION == SFRAME_VERSION_3)
     {
@@ -155,6 +155,10 @@  sframe_fde_tbl_init (sf_fde_tbl *fde_tbl, const char *fde_buf,
 	  fde_tbl->entry[i].func_start_fre_off = fdep->sfdi_func_start_fre_off;
 	  /* V3 organizes the following data closer to the SFrame FREs for the
 	     function.  Access them via the sfde_func_start_fre_off.  */
+	  if (fdep->sfdi_func_start_fre_off > fre_buf_len
+	      || (sizeof (sframe_func_desc_attr_v3)
+		  > (fre_buf_len - fdep->sfdi_func_start_fre_off)))
+	    return SFRAME_ERR;
 	  const sframe_func_desc_attr_v3 *fattr
 	    = (sframe_func_desc_attr_v3 *)(fre_buf
 					   + fdep->sfdi_func_start_fre_off);
@@ -1535,7 +1539,7 @@  sframe_decode (const char *sf_buf, size_t sf_size, int *errp)
 
   /* SFrame FDEs are at an offset of sfh_fdeoff from SFrame header end.  */
   if (sframe_fde_tbl_init (dctx->sfd_funcdesc, frame_buf + dhp->sfh_fdeoff,
-			   frame_buf + dhp->sfh_freoff,
+			   frame_buf + dhp->sfh_freoff, dhp->sfh_fre_len,
 			   &fidx_size, dhp->sfh_num_fdes, sfp->sfp_version))
     {
       sframe_ret_set_errno (errp, SFRAME_ERR_BUF_INVAL);