| Message ID | 20240626233845.151197-3-pablo@netfilter.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7931d32955e09d0a11b1fe0b6aac1bfa061c005c |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] netfilter: fix undefined reference to 'netfilter_lwtunnel_*' when CONFIG_SYSCTL=n | expand |
On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> Oh, I was only the messenger boy, not the actual reporter. I think reporting credit should probably go to HexRabbit Chen <hexrabbit@devco.re> Linus
On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > Oh, I was only the messenger boy, not the actual reporter. > > I think reporting credit should probably go to HexRabbit Chen > <hexrabbit@devco.re> I would not have really know if you don't tell me TBH, else it would have taken even longer for me to react and fix it. Because they did not really contact me to report this issue this time. But if you insist, I will do so. Thanks.
Hi, On Thu, 2024-06-27 at 03:13 +0200, Pablo Neira Ayuso wrote: > On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > > > Oh, I was only the messenger boy, not the actual reporter. > > > > I think reporting credit should probably go to HexRabbit Chen > > <hexrabbit@devco.re> > > I would not have really know if you don't tell me TBH, else it would > have taken even longer for me to react and fix it. Because they did > not really contact me to report this issue this time. > > But if you insist, I will do so. I'm sorry for the late reply. I guess the most fair option would be adding both tags. With a repost, this will not make it into todays PR, I hope it's not a problem. Thanks, Paolo
On Thu, Jun 27, 2024 at 12:26:49PM +0200, Paolo Abeni wrote: > Hi, > On Thu, 2024-06-27 at 03:13 +0200, Pablo Neira Ayuso wrote: > > On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > > > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > > > > > Oh, I was only the messenger boy, not the actual reporter. > > > > > > I think reporting credit should probably go to HexRabbit Chen > > > <hexrabbit@devco.re> > > > > I would not have really know if you don't tell me TBH, else it would > > have taken even longer for me to react and fix it. Because they did > > not really contact me to report this issue this time. > > > > But if you insist, I will do so. > > I'm sorry for the late reply. > > I guess the most fair option would be adding both tags. > > With a repost, this will not make it into todays PR, I hope it's not a > problem. It is a addressing a public issue, the reporter decided to follow a different channel other than security@ for whatever reason. I'd prefer if you can take it in this round.
On Thu, 2024-06-27 at 12:29 +0200, Pablo Neira Ayuso wrote: > On Thu, Jun 27, 2024 at 12:26:49PM +0200, Paolo Abeni wrote: > > On Thu, 2024-06-27 at 03:13 +0200, Pablo Neira Ayuso wrote: > > > On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > > > > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > > > > > > > Oh, I was only the messenger boy, not the actual reporter. > > > > > > > > I think reporting credit should probably go to HexRabbit Chen > > > > <hexrabbit@devco.re> > > > > > > I would not have really know if you don't tell me TBH, else it would > > > have taken even longer for me to react and fix it. Because they did > > > not really contact me to report this issue this time. > > > > > > But if you insist, I will do so. > > > > I'm sorry for the late reply. > > > > I guess the most fair option would be adding both tags. > > > > With a repost, this will not make it into todays PR, I hope it's not a > > problem. > > It is a addressing a public issue, the reporter decided to follow a > different channel other than security@ for whatever reason. > > I'd prefer if you can take it in this round. Sure, we are still (barely ;) on time! Thanks for the prompt feedback. Paolo
On Thu, Jun 27, 2024 at 12:29:20PM +0200, Pablo Neira Ayuso wrote: > On Thu, Jun 27, 2024 at 12:26:49PM +0200, Paolo Abeni wrote: > > Hi, > > On Thu, 2024-06-27 at 03:13 +0200, Pablo Neira Ayuso wrote: > > > On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > > > > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > > > > > > > Oh, I was only the messenger boy, not the actual reporter. > > > > > > > > I think reporting credit should probably go to HexRabbit Chen > > > > <hexrabbit@devco.re> > > > > > > I would not have really know if you don't tell me TBH, else it would > > > have taken even longer for me to react and fix it. Because they did > > > not really contact me to report this issue this time. > > > > > > But if you insist, I will do so. > > > > I'm sorry for the late reply. > > > > I guess the most fair option would be adding both tags. "Fair option" maybe sounds too strong in this case, that email which reported this pointer leak to userspace through ZDI did not even report this issue to us in first place... Linus was so kind to attract my attention on this, I appreciate he contacted me. Could you append this text to the pull request message: Linus found this pointer leak to userspace via zdi-disclosures@ and forwarded the notice to Netfilter maintainers, he appears as reporter because whoever found this issue never approached Netfilter maintainers neither via security@ nor in private. If still not acceptable, I am fine to send a new PR and miss this round of fixes. Thanks Paolo. > > With a repost, this will not make it into todays PR, I hope it's not a > > problem. > > It is a addressing a public issue, the reporter decided to follow a > different channel other than security@ for whatever reason. > > I'd prefer if you can take it in this round.
On Thu, Jun 27, 2024 at 12:37:59PM +0200, Paolo Abeni wrote: > On Thu, 2024-06-27 at 12:29 +0200, Pablo Neira Ayuso wrote: > > On Thu, Jun 27, 2024 at 12:26:49PM +0200, Paolo Abeni wrote: > > > On Thu, 2024-06-27 at 03:13 +0200, Pablo Neira Ayuso wrote: > > > > On Wed, Jun 26, 2024 at 05:51:13PM -0700, Linus Torvalds wrote: > > > > > On Wed, 26 Jun 2024 at 16:38, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > > > > > > > Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> > > > > > > > > > > Oh, I was only the messenger boy, not the actual reporter. > > > > > > > > > > I think reporting credit should probably go to HexRabbit Chen > > > > > <hexrabbit@devco.re> > > > > > > > > I would not have really know if you don't tell me TBH, else it would > > > > have taken even longer for me to react and fix it. Because they did > > > > not really contact me to report this issue this time. > > > > > > > > But if you insist, I will do so. > > > > > > I'm sorry for the late reply. > > > > > > I guess the most fair option would be adding both tags. > > > > > > With a repost, this will not make it into todays PR, I hope it's not a > > > problem. > > > > It is a addressing a public issue, the reporter decided to follow a > > different channel other than security@ for whatever reason. > > > > I'd prefer if you can take it in this round. > > Sure, we are still (barely ;) on time! > > Thanks for the prompt feedback. Thanks a lot Paolo!
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h index 2796153b03da..188d41da1a40 100644 --- a/include/net/netfilter/nf_tables.h +++ b/include/net/netfilter/nf_tables.h @@ -619,6 +619,11 @@ static inline void *nft_set_priv(const struct nft_set *set) return (void *)set->data; } +static inline enum nft_data_types nft_set_datatype(const struct nft_set *set) +{ + return set->dtype == NFT_DATA_VERDICT ? NFT_DATA_VERDICT : NFT_DATA_VALUE; +} + static inline bool nft_set_gc_is_pending(const struct nft_set *s) { return refcount_read(&s->refs) != 1; diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index be3b4c90d2ed..e8dcf41d360d 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -5740,8 +5740,7 @@ static int nf_tables_fill_setelem(struct sk_buff *skb, if (nft_set_ext_exists(ext, NFT_SET_EXT_DATA) && nft_data_dump(skb, NFTA_SET_ELEM_DATA, nft_set_ext_data(ext), - set->dtype == NFT_DATA_VERDICT ? NFT_DATA_VERDICT : NFT_DATA_VALUE, - set->dlen) < 0) + nft_set_datatype(set), set->dlen) < 0) goto nla_put_failure; if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPRESSIONS) && @@ -11073,6 +11072,9 @@ static int nft_validate_register_store(const struct nft_ctx *ctx, return 0; default: + if (type != NFT_DATA_VALUE) + return -EINVAL; + if (reg < NFT_REG_1 * NFT_REG_SIZE / NFT_REG32_SIZE) return -EINVAL; if (len == 0) @@ -11081,8 +11083,6 @@ static int nft_validate_register_store(const struct nft_ctx *ctx, sizeof_field(struct nft_regs, data)) return -ERANGE; - if (data != NULL && type != NFT_DATA_VALUE) - return -EINVAL; return 0; } } diff --git a/net/netfilter/nft_lookup.c b/net/netfilter/nft_lookup.c index b314ca728a29..f3080fa1b226 100644 --- a/net/netfilter/nft_lookup.c +++ b/net/netfilter/nft_lookup.c @@ -132,7 +132,8 @@ static int nft_lookup_init(const struct nft_ctx *ctx, return -EINVAL; err = nft_parse_register_store(ctx, tb[NFTA_LOOKUP_DREG], - &priv->dreg, NULL, set->dtype, + &priv->dreg, NULL, + nft_set_datatype(set), set->dlen); if (err < 0) return err;
register store validation for NFT_DATA_VALUE is conditional, however, the datatype is always either NFT_DATA_VALUE or NFT_DATA_VERDICT. This only requires a new helper function to infer the register type from the set datatype so this conditional check can be removed. Otherwise, pointer to chain object can be leaked through the registers. Fixes: 96518518cc41 ("netfilter: add nftables") Reported-by: Linus Torvalds <torvalds@linuxfoundation.org> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- include/net/netfilter/nf_tables.h | 5 +++++ net/netfilter/nf_tables_api.c | 8 ++++---- net/netfilter/nft_lookup.c | 3 ++- 3 files changed, 11 insertions(+), 5 deletions(-)