diff mbox series

[net-next] net/sched: clear actions pointer in miss cookie init fail

Message ID 20230414214317.227128-1-pctammela@mojatatu.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net/sched: clear actions pointer in miss cookie init fail | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 27 this patch: 27
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 27 this patch: 27
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pedro Tammela April 14, 2023, 9:43 p.m. UTC
Palash reports a UAF when using a modified version of syzkaller[1].

When 'tcf_exts_miss_cookie_base_alloc()' fails in 'tcf_exts_init_ex()'
a call to 'tcf_exts_destroy()' is made to free up the tcf_exts
resources.
In flower, a call to '__fl_put()' when 'tcf_exts_init_ex()' fails is made;
Then calling 'tcf_exts_destroy()', which triggers an UAF since the
already freed tcf_exts action pointer is lingering in the struct.

Before the offending patch, this was not an issue since there was no
case where the tcf_exts action pointer could linger. Therefore, restore
the old semantic by clearing the action pointer in case of a failure to
initialize the miss_cookie.

[1] https://github.com/cmu-pasta/linux-kernel-enriched-corpus

Cc: paulb@nvidia.com
Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action")
Reported-by: Palash Oswal <oswalpalash@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/sched/cls_api.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Pedro Tammela April 14, 2023, 9:44 p.m. UTC | #1
On 14/04/2023 18:43, Pedro Tammela wrote:
> Palash reports a UAF when using a modified version of syzkaller[1].
> 
> When 'tcf_exts_miss_cookie_base_alloc()' fails in 'tcf_exts_init_ex()'
> a call to 'tcf_exts_destroy()' is made to free up the tcf_exts
> resources.
> In flower, a call to '__fl_put()' when 'tcf_exts_init_ex()' fails is made;
> Then calling 'tcf_exts_destroy()', which triggers an UAF since the
> already freed tcf_exts action pointer is lingering in the struct.
> 
> Before the offending patch, this was not an issue since there was no
> case where the tcf_exts action pointer could linger. Therefore, restore
> the old semantic by clearing the action pointer in case of a failure to
> initialize the miss_cookie.
> 
> [1] https://github.com/cmu-pasta/linux-kernel-enriched-corpus
> 
> Cc: paulb@nvidia.com
> Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action")
> Reported-by: Palash Oswal <oswalpalash@gmail.com>
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>   net/sched/cls_api.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 2a6b6be0811b..84bad268e328 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -3235,6 +3235,7 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action,
>   
>   err_miss_alloc:
>   	tcf_exts_destroy(exts);
> +	exts->actions = NULL;
>   	return err;
>   }
>   EXPORT_SYMBOL(tcf_exts_init_ex);


My bad, this should target 'net' instead of 'net-next'.
kernel test robot April 15, 2023, 6:45 a.m. UTC | #2
Hi Pedro,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Pedro-Tammela/net-sched-clear-actions-pointer-in-miss-cookie-init-fail/20230415-054434
patch link:    https://lore.kernel.org/r/20230414214317.227128-1-pctammela%40mojatatu.com
patch subject: [PATCH net-next] net/sched: clear actions pointer in miss cookie init fail
config: riscv-defconfig (https://download.01.org/0day-ci/archive/20230415/202304151408.D1kAjGwb-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0a51c6cee30eab6b3023dbcd65899511f14cd8e8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Pedro-Tammela/net-sched-clear-actions-pointer-in-miss-cookie-init-fail/20230415-054434
        git checkout 0a51c6cee30eab6b3023dbcd65899511f14cd8e8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/sched/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304151408.D1kAjGwb-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/sched/cls_api.c: In function 'tcf_exts_init_ex':
>> net/sched/cls_api.c:3238:15: error: 'struct tcf_exts' has no member named 'actions'; did you mean 'action'?
    3238 |         exts->actions = NULL;
         |               ^~~~~~~
         |               action


vim +3238 net/sched/cls_api.c

  3223	
  3224		exts->action = action;
  3225		exts->police = police;
  3226	
  3227		if (!use_action_miss)
  3228			return 0;
  3229	
  3230		err = tcf_exts_miss_cookie_base_alloc(exts, tp, handle);
  3231		if (err)
  3232			goto err_miss_alloc;
  3233	
  3234		return 0;
  3235	
  3236	err_miss_alloc:
  3237		tcf_exts_destroy(exts);
> 3238		exts->actions = NULL;
  3239		return err;
  3240	}
  3241	EXPORT_SYMBOL(tcf_exts_init_ex);
  3242
diff mbox series

Patch

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 2a6b6be0811b..84bad268e328 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -3235,6 +3235,7 @@  int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action,
 
 err_miss_alloc:
 	tcf_exts_destroy(exts);
+	exts->actions = NULL;
 	return err;
 }
 EXPORT_SYMBOL(tcf_exts_init_ex);