neighbour: complement nl_ntbl_parm_policy

[PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by Lin Ma 1 year, 11 months ago

In the neightbl_set function, the attributes array is parsed and validated
using the nl_ntbl_parm_policy policy. However, this policy overlooks the
NDTPA_QUEUE_LENBYTES attribute since the commit 6b3f8674bccb ("[NEIGH]:
Convert neighbour table modification to new netlink api").
As a result, no validation is performed when accessing the
NDTPA_QUEUE_LENBYTES attribute.

This patch addresses this issue by complementing the policy to ensure that
every attribute being accessed is properly validated.

Signed-off-by: Lin Ma <linma@zju.edu.cn>
---
 net/core/neighbour.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 552719c3bbc3..ece0447cf409 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2293,6 +2293,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
 static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
 	[NDTPA_IFINDEX]			= { .type = NLA_U32 },
 	[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },
+	[NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
 	[NDTPA_PROXY_QLEN]		= { .type = NLA_U32 },
 	[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
 	[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
-- 
2.34.1

Re: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by kernel test robot 1 year, 11 months ago

Hi Lin,

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/Lin-Ma/neighbour-complement-nl_ntbl_parm_policy/20240119-151255
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240119070847.5402-1-linma%40zju.edu.cn
patch subject: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240120/202401202347.sHNru6Y6-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401202347.sHNru6Y6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401202347.sHNru6Y6-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/core/neighbour.c:2296:10: error: 'NPTPA_QUEUE_LEN_BYTES' undeclared here (not in a function); did you mean 'NDTPA_QUEUE_LENBYTES'?
    2296 |         [NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
         |          ^~~~~~~~~~~~~~~~~~~~~
         |          NDTPA_QUEUE_LENBYTES
>> net/core/neighbour.c:2296:10: error: array index in initializer not of integer type
   net/core/neighbour.c:2296:10: note: (near initialization for 'nl_ntbl_parm_policy')
   net/core/neighbour.c:2298:43: warning: initialized field overwritten [-Woverride-init]
    2298 |         [NDTPA_APP_PROBES]              = { .type = NLA_U32 },
         |                                           ^
   net/core/neighbour.c:2298:43: note: (near initialization for 'nl_ntbl_parm_policy[9]')


vim +2296 net/core/neighbour.c

  2292	
  2293	static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
  2294		[NDTPA_IFINDEX]			= { .type = NLA_U32 },
  2295		[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },
> 2296		[NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
  2297		[NDTPA_PROXY_QLEN]		= { .type = NLA_U32 },
  2298		[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
  2299		[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
  2300		[NDTPA_MCAST_PROBES]		= { .type = NLA_U32 },
  2301		[NDTPA_MCAST_REPROBES]		= { .type = NLA_U32 },
  2302		[NDTPA_BASE_REACHABLE_TIME]	= { .type = NLA_U64 },
  2303		[NDTPA_GC_STALETIME]		= { .type = NLA_U64 },
  2304		[NDTPA_DELAY_PROBE_TIME]	= { .type = NLA_U64 },
  2305		[NDTPA_RETRANS_TIME]		= { .type = NLA_U64 },
  2306		[NDTPA_ANYCAST_DELAY]		= { .type = NLA_U64 },
  2307		[NDTPA_PROXY_DELAY]		= { .type = NLA_U64 },
  2308		[NDTPA_LOCKTIME]		= { .type = NLA_U64 },
  2309		[NDTPA_INTERVAL_PROBE_TIME_MS]	= { .type = NLA_U64, .min = 1 },
  2310	};
  2311	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by kernel test robot 1 year, 11 months ago

Hi Lin,

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/Lin-Ma/neighbour-complement-nl_ntbl_parm_policy/20240119-151255
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240119070847.5402-1-linma%40zju.edu.cn
patch subject: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy
config: arm-randconfig-001-20240120 (https://download.01.org/0day-ci/archive/20240120/202401200717.gbJdfFML-lkp@intel.com/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d92ce344bf641e6bb025b41b3f1a77dd25e2b3e9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200717.gbJdfFML-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401200717.gbJdfFML-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/core/neighbour.c:2296:3: error: use of undeclared identifier 'NPTPA_QUEUE_LEN_BYTES'; did you mean 'NDTPA_QUEUE_LENBYTES'?
    2296 |         [NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
         |          ^~~~~~~~~~~~~~~~~~~~~
         |          NDTPA_QUEUE_LENBYTES
   include/uapi/linux/neighbour.h:160:2: note: 'NDTPA_QUEUE_LENBYTES' declared here
     160 |         NDTPA_QUEUE_LENBYTES,           /* u32 */
         |         ^
   1 error generated.


vim +2296 net/core/neighbour.c

  2292	
  2293	static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
  2294		[NDTPA_IFINDEX]			= { .type = NLA_U32 },
  2295		[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },
> 2296		[NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
  2297		[NDTPA_PROXY_QLEN]		= { .type = NLA_U32 },
  2298		[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
  2299		[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
  2300		[NDTPA_MCAST_PROBES]		= { .type = NLA_U32 },
  2301		[NDTPA_MCAST_REPROBES]		= { .type = NLA_U32 },
  2302		[NDTPA_BASE_REACHABLE_TIME]	= { .type = NLA_U64 },
  2303		[NDTPA_GC_STALETIME]		= { .type = NLA_U64 },
  2304		[NDTPA_DELAY_PROBE_TIME]	= { .type = NLA_U64 },
  2305		[NDTPA_RETRANS_TIME]		= { .type = NLA_U64 },
  2306		[NDTPA_ANYCAST_DELAY]		= { .type = NLA_U64 },
  2307		[NDTPA_PROXY_DELAY]		= { .type = NLA_U64 },
  2308		[NDTPA_LOCKTIME]		= { .type = NLA_U64 },
  2309		[NDTPA_INTERVAL_PROBE_TIME_MS]	= { .type = NLA_U64, .min = 1 },
  2310	};
  2311	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by Simon Horman 1 year, 11 months ago

On Fri, Jan 19, 2024 at 03:08:47PM +0800, Lin Ma wrote:
> In the neightbl_set function, the attributes array is parsed and validated
> using the nl_ntbl_parm_policy policy. However, this policy overlooks the
> NDTPA_QUEUE_LENBYTES attribute since the commit 6b3f8674bccb ("[NEIGH]:
> Convert neighbour table modification to new netlink api").
> As a result, no validation is performed when accessing the
> NDTPA_QUEUE_LENBYTES attribute.
> 
> This patch addresses this issue by complementing the policy to ensure that
> every attribute being accessed is properly validated.
> 
> Signed-off-by: Lin Ma <linma@zju.edu.cn>
> ---
>  net/core/neighbour.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 552719c3bbc3..ece0447cf409 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2293,6 +2293,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
>  static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
>  	[NDTPA_IFINDEX]			= { .type = NLA_U32 },
>  	[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },
> +	[NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },

This does not compile because NPTPA_QUEUE_LEN_BYTES is
not present in net-next.

>  	[NDTPA_PROXY_QLEN]		= { .type = NLA_U32 },
>  	[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
>  	[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },


## Form letter - net-next-closed

[adapted from text by Jakub]

The merge window for v6.8 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens on or after 22nd January.

RFC patches sent for review only are obviously welcome at any time.

See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer

Re: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by Lin Ma 1 year, 11 months ago

Hello Simon,

> On Fri, Jan 19, 2024 at 03:08:47PM +0800, Lin Ma wrote:
> > In the neightbl_set function, the attributes array is parsed and validated
> > using the nl_ntbl_parm_policy policy. However, this policy overlooks the
> > NDTPA_QUEUE_LENBYTES attribute since the commit 6b3f8674bccb ("[NEIGH]:
> > Convert neighbour table modification to new netlink api").
> > As a result, no validation is performed when accessing the
> > NDTPA_QUEUE_LENBYTES attribute.
> > 
> > This patch addresses this issue by complementing the policy to ensure that
> > every attribute being accessed is properly validated.
> > 
> > Signed-off-by: Lin Ma <linma@zju.edu.cn>
> > ---
> >  net/core/neighbour.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 552719c3bbc3..ece0447cf409 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -2293,6 +2293,7 @@ static const struct nla_policy nl_neightbl_policy[NDTA_MAX+1] = {
> >  static const struct nla_policy nl_ntbl_parm_policy[NDTPA_MAX+1] = {
> >  	[NDTPA_IFINDEX]			= { .type = NLA_U32 },
> >  	[NDTPA_QUEUE_LEN]		= { .type = NLA_U32 },
> > +	[NPTPA_QUEUE_LEN_BYTES]         = { .type = NLA_U32 },
> 
> This does not compile because NPTPA_QUEUE_LEN_BYTES is
> not present in net-next.
> 
> >  	[NDTPA_PROXY_QLEN]		= { .type = NLA_U32 },
> >  	[NDTPA_APP_PROBES]		= { .type = NLA_U32 },
> >  	[NDTPA_UCAST_PROBES]		= { .type = NLA_U32 },
> 
> 
> ## Form letter - net-next-closed
> 
> [adapted from text by Jakub]
> 
> The merge window for v6.8 has begun and therefore net-next is closed
> for new drivers, features, code refactoring and optimizations.
> We are currently accepting bug fixes only.
> 
> Please repost when net-next reopens on or after 22nd January.
> 
> RFC patches sent for review only are obviously welcome at any time.
> 
> See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
> --
> pw-bot: defer

My bad, I prepare this patch on the linux-stable tree and never thought this would happen.
Will also compile on the right tree next time.

So should I send this to net which has this attribute or something?

Thanks are really Sorry
Lin

Re: [PATCH net-next v1] neighbour: complement nl_ntbl_parm_policy

Posted by Simon Horman 1 year, 11 months ago

On Sat, Jan 20, 2024 at 08:53:50AM +0800, Lin Ma wrote:
> Hello Simon,

...

> My bad, I prepare this patch on the linux-stable tree and never thought this would happen.
> Will also compile on the right tree next time.
> 
> So should I send this to net which has this attribute or something?

Hi Lin Ma,

If it is a fix for net, then it should be based on net and targeted at net.

	Subject: [PATCH net] ...

Else, it should be based on and targeted at net-next.

Either way, I think reposting is a good idea.
Although in the net-next case, please note that it is closed,
so please wait for it to reopen before posting patches for it.
It is likely to reopen next week.

Does that help?