[RFC] [PATCH] netfilter: nfnetlink_queue: Don't set random flag bits

[RFC] [PATCH] netfilter: nfnetlink_queue: Don't set random flag bits

[Krishna Kumar]

Allow setting of only supported flag bits in queue->flags.

If this is OK, I can send a patch to add this flag to
libnetfilter_queue too.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 include/linux/netfilter/nfnetlink_queue.h |    2 ++
 net/netfilter/nfnetlink_queue_core.c      |    6 ++++++
 2 files changed, 8 insertions(+)

diff -ruNp org/include/linux/netfilter/nfnetlink_queue.h new/include/linux/netfilter/nfnetlink_queue.h
--- org/include/linux/netfilter/nfnetlink_queue.h	2012-06-18 08:36:53.000000000 +0530
+++ new/include/linux/netfilter/nfnetlink_queue.h	2012-06-27 16:25:54.297619352 +0530
@@ -96,4 +96,6 @@ enum nfqnl_attr_config {
 #define NFQA_CFG_F_FAIL_OPEN			(1 << 0)
 #define NFQA_CFG_F_CONNTRACK			(1 << 1)
 
+#define NFQA_CFG_F_FLAGS_MAX			(1 << 2)
+
 #endif /* _NFNETLINK_QUEUE_H */
diff -ruNp org/net/netfilter/nfnetlink_queue_core.c new/net/netfilter/nfnetlink_queue_core.c
--- org/net/netfilter/nfnetlink_queue_core.c	2012-06-27 12:34:02.000000000 +0530
+++ new/net/netfilter/nfnetlink_queue_core.c	2012-06-27 14:00:46.153670918 +0530
@@ -910,6 +910,12 @@ nfqnl_recv_config(struct sock *ctnl, str
 		flags = ntohl(nla_get_be32(nfqa[NFQA_CFG_FLAGS]));
 		mask = ntohl(nla_get_be32(nfqa[NFQA_CFG_MASK]));
 
+		if (flags >= NFQA_CFG_F_FLAGS_MAX) {
+			/* flags has more bits than what is supported */
+			ret = -EOPNOTSUPP;
+			goto err_out_unlock;
+		}
+
 		spin_lock_bh(&queue->lock);
 		queue->flags &= ~mask;
 		queue->flags |= flags & mask;

Re: [RFC] [PATCH] netfilter: nfnetlink_queue: Don't set random flag bits

[Pablo Neira Ayuso]

On Wed, Jun 27, 2012 at 04:29:56PM +0530, Krishna Kumar wrote:
> Allow setting of only supported flag bits in queue->flags.

I have applied this with minor glitches, see below.

> If this is OK, I can send a patch to add this flag to
> libnetfilter_queue too.

Not sure what you mean with the patch for libnetfilter_queue. If you
consider adding some checking in user-space, no need to do it. We have
to fail back on the kernel to report this. So we don't need to update
libnetfilter_queue if we get new flags supported.

Back to your patch...

> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
> ---
>  include/linux/netfilter/nfnetlink_queue.h |    2 ++
>  net/netfilter/nfnetlink_queue_core.c      |    6 ++++++
>  2 files changed, 8 insertions(+)
> 
> diff -ruNp org/include/linux/netfilter/nfnetlink_queue.h new/include/linux/netfilter/nfnetlink_queue.h
> --- org/include/linux/netfilter/nfnetlink_queue.h	2012-06-18 08:36:53.000000000 +0530
> +++ new/include/linux/netfilter/nfnetlink_queue.h	2012-06-27 16:25:54.297619352 +0530
> @@ -96,4 +96,6 @@ enum nfqnl_attr_config {
>  #define NFQA_CFG_F_FAIL_OPEN			(1 << 0)
>  #define NFQA_CFG_F_CONNTRACK			(1 << 1)
>  

removed this extra space above.

> +#define NFQA_CFG_F_FLAGS_MAX			(1 << 2)

removed _FLAGS, it seems redundant to _F.

> +
>  #endif /* _NFNETLINK_QUEUE_H */
> diff -ruNp org/net/netfilter/nfnetlink_queue_core.c new/net/netfilter/nfnetlink_queue_core.c
> --- org/net/netfilter/nfnetlink_queue_core.c	2012-06-27 12:34:02.000000000 +0530
> +++ new/net/netfilter/nfnetlink_queue_core.c	2012-06-27 14:00:46.153670918 +0530
> @@ -910,6 +910,12 @@ nfqnl_recv_config(struct sock *ctnl, str
>  		flags = ntohl(nla_get_be32(nfqa[NFQA_CFG_FLAGS]));
>  		mask = ntohl(nla_get_be32(nfqa[NFQA_CFG_MASK]));
>  
> +		if (flags >= NFQA_CFG_F_FLAGS_MAX) {
> +			/* flags has more bits than what is supported */

removed this comment, I think it's more or less evinder to the source
code reader what this does.

> +			ret = -EOPNOTSUPP;
> +			goto err_out_unlock;
> +		}
> +
>  		spin_lock_bh(&queue->lock);
>  		queue->flags &= ~mask;
>  		queue->flags |= flags & mask;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html