Skip to contentSkip to conversation

netfilter: fix - mistake to pass a pointer where array is required

PATCH In review lizf 2 participants 6 messages Oct 12, 2007 – Oct 18, 2007

Subsystems (2)
lore
Share & tools
lore

lizf

author
#0 lore raw

PATCH 1/2 netfilter: fix - mistake to pass a pointer where array is required

Hi,

Macros like SCTP_CHUNKMAP_XXX(chukmap) require chukmap to be an array, We can see from below:

#define ELEMCOUNT(x) (sizeof(x)/sizeof(x[0]))

#define SCTP_CHUNKMAP_RESET(chunkmap)				\
	do {							\
		int i;						\
		for (i = 0; i < ELEMCOUNT(chunkmap); i++)	\
			chunkmap[i] = 0;			\
	} while (0)

But match_packet() passes a pointer to these macros. Here's the patch.

net/netfilter/xt_sctp.c +0 -0
index c002153..f65c71c 100644
@@ -41,21 +41,22 @@ match_flags(const struct xt_sctp_flag_info *flag_info,

static inline bool
match_packet(const struct sk_buff *skb,
unsigned int offset,

-	     const u_int32_t *chunkmap,
-	     int chunk_match_type,
-	     const struct xt_sctp_flag_info *flag_info,
-	     const int flag_count,
+	     const struct xt_sctp_info *info,
	     bool *hotdrop)
{
	u_int32_t chunkmapcopy[256 / sizeof (u_int32_t)];
	sctp_chunkhdr_t _sch, *sch;

+ int chunk_match_type = info->chunk_match_type;
+ const struct xt_sctp_flag_info *flag_info = info->flag_info;
+ int flag_count = info->flag_count;
+

#ifdef DEBUG_SCTP
	int i = 0;

#endif

	if (chunk_match_type == SCTP_CHUNK_MATCH_ALL)

- SCTP_CHUNKMAP_COPY(chunkmapcopy, chunkmap);
+ SCTP_CHUNKMAP_COPY(chunkmapcopy, info->chunkmap);

	do {
		sch = skb_header_pointer(skb, offset, sizeof(_sch), &_sch);

@@ -72,7 +73,7 @@ match_packet(const struct sk_buff *skb,

		duprintf("skb->len: %d\toffset: %d\n", skb->len, offset);

- if (SCTP_CHUNKMAP_IS_SET(chunkmap, sch->type)) {

+		if (SCTP_CHUNKMAP_IS_SET(info->chunkmap, sch->type)) {
			switch (chunk_match_type) {
			case SCTP_CHUNK_MATCH_ANY:
				if (match_flags(flag_info, flag_count,

@@ -103,7 +104,7 @@ match_packet(const struct sk_buff *skb,

	switch (chunk_match_type) {
	case SCTP_CHUNK_MATCH_ALL:
-		return SCTP_CHUNKMAP_IS_CLEAR(chunkmap);
+		return SCTP_CHUNKMAP_IS_CLEAR(info->chunkmap);
	case SCTP_CHUNK_MATCH_ANY:
		return false;
	case SCTP_CHUNK_MATCH_ONLY:
@@ -147,9 +148,7 @@ match(const struct sk_buff *skb,
			&& ntohs(sh->dest) <= info->dpts[1],
			XT_SCTP_DEST_PORTS, info->flags, info->invflags)
		&& SCCHECK(match_packet(skb, protoff + sizeof (sctp_sctphdr_t),

- info->chunkmap, info->chunk_match_type,
- info->flag_info, info->flag_count,
- hotdrop),
+ info, hotdrop),

			   XT_SCTP_CHUNK_TYPES, info->flags, info->invflags);
}
#1 lore raw
Quoted 14 lines · lizf

Hi,

Macros like SCTP_CHUNKMAP_XXX(chukmap) require chukmap to be an array, We can see from below:

#define ELEMCOUNT(x) (sizeof(x)/sizeof(x[0]))

#define SCTP_CHUNKMAP_RESET(chunkmap)                \
    do {                            \
        int i;                        \
        for (i = 0; i < ELEMCOUNT(chunkmap); i++)    \
            chunkmap[i] = 0;            \
    } while (0)
Quoted 3 lines

But match_packet() passes a pointer to these macros. Here's the patch.

Thanks, but your mailer corrupted the patch by converting tabs to spaces. Please resend as attachment so I can apply it.

author
#2 lore raw
Quoted 12 lines · Patrick McHardy
Quoted 12 lines

Macros like SCTP_CHUNKMAP_XXX(chukmap) require chukmap to be an array, We can see from below:

#define ELEMCOUNT(x) (sizeof(x)/sizeof(x[0]))

#define SCTP_CHUNKMAP_RESET(chunkmap)                \
    do {                            \
        int i;                        \
        for (i = 0; i < ELEMCOUNT(chunkmap); i++)    \
            chunkmap[i] = 0;            \
    } while (0)
Quoted 9 lines
Quoted 3 lines

But match_packet() passes a pointer to these macros. Here's the patch.

Thanks, but your mailer corrupted the patch by converting tabs to spaces. Please resend as attachment so I can apply it.

Sorry for the noise, I've configed my mailer according to a kernel doc, but i'm still not sure whether it works. So here's the attachment.

Li Zefan

net/netfilter/xt_sctp.c +9 -10
index c002153..f65c71c 100644
@@ -41,21 +41,22 @@ match_flags(const struct xt_sctp_flag_info *flag_info,
 static inline bool
 match_packet(const struct sk_buff *skb,
 	     unsigned int offset,
-	     const u_int32_t *chunkmap,
-	     int chunk_match_type,
-	     const struct xt_sctp_flag_info *flag_info,
-	     const int flag_count,
+	     const struct xt_sctp_info *info,
 	     bool *hotdrop)
 {
 	u_int32_t chunkmapcopy[256 / sizeof (u_int32_t)];
 	sctp_chunkhdr_t _sch, *sch;
 
+	int chunk_match_type = info->chunk_match_type;
+	const struct xt_sctp_flag_info *flag_info = info->flag_info;
+	int flag_count = info->flag_count;
+
 #ifdef DEBUG_SCTP
 	int i = 0;
 #endif
 
 	if (chunk_match_type == SCTP_CHUNK_MATCH_ALL)
-		SCTP_CHUNKMAP_COPY(chunkmapcopy, chunkmap);
+		SCTP_CHUNKMAP_COPY(chunkmapcopy, info->chunkmap);
 
 	do {
 		sch = skb_header_pointer(skb, offset, sizeof(_sch), &_sch);
@@ -72,7 +73,7 @@ match_packet(const struct sk_buff *skb,
 
 		duprintf("skb->len: %d\toffset: %d\n", skb->len, offset);
 
-		if (SCTP_CHUNKMAP_IS_SET(chunkmap, sch->type)) {
+		if (SCTP_CHUNKMAP_IS_SET(info->chunkmap, sch->type)) {
 			switch (chunk_match_type) {
 			case SCTP_CHUNK_MATCH_ANY:
 				if (match_flags(flag_info, flag_count,
@@ -103,7 +104,7 @@ match_packet(const struct sk_buff *skb,
 
 	switch (chunk_match_type) {
 	case SCTP_CHUNK_MATCH_ALL:
-		return SCTP_CHUNKMAP_IS_CLEAR(chunkmap);
+		return SCTP_CHUNKMAP_IS_CLEAR(info->chunkmap);
 	case SCTP_CHUNK_MATCH_ANY:
 		return false;
 	case SCTP_CHUNK_MATCH_ONLY:
@@ -147,9 +148,7 @@ match(const struct sk_buff *skb,
 			&& ntohs(sh->dest) <= info->dpts[1],
 			XT_SCTP_DEST_PORTS, info->flags, info->invflags)
 		&& SCCHECK(match_packet(skb, protoff + sizeof (sctp_sctphdr_t),
-					info->chunkmap, info->chunk_match_type,
-					info->flag_info, info->flag_count,
-					hotdrop),
+					info, hotdrop),
 			   XT_SCTP_CHUNK_TYPES, info->flags, info->invflags);
 }
 
#3 lore raw
Quoted 3 lines · lizf

Sorry for the noise, I've configed my mailer according to a kernel doc, but i'm still not sure whether it works. So here's the attachment.

Thanks, the attachment applies fine, but does not compile:

CC [M] net/netfilter/xt_sctp.o
net/netfilter/xt_sctp.c: In function 'match_packet':
net/netfilter/xt_sctp.c:60: error: 'chunkmap' undeclared (first use in this function) net/netfilter/xt_sctp.c:60: error: (Each undeclared identifier is reported only once net/netfilter/xt_sctp.c:60: error: for each function it appears in.) make[1]: *** [net/netfilter/xt_sctp.o] Error 1
make: *** [net/netfilter/xt_sctp.o] Error 2

The SCTP_CHUNKMAP_COPY uses "chunkmap" internally. What tree did you base this patch on?

#4 lore raw
Quoted 16 lines · Patrick McHardy
Quoted 3 lines · lizf

Sorry for the noise, I've configed my mailer according to a kernel doc, but i'm still not sure whether it works. So here's the attachment.

Thanks, the attachment applies fine, but does not compile:

CC [M] net/netfilter/xt_sctp.o
net/netfilter/xt_sctp.c: In function 'match_packet':
net/netfilter/xt_sctp.c:60: error: 'chunkmap' undeclared (first use in this function) net/netfilter/xt_sctp.c:60: error: (Each undeclared identifier is reported only once net/netfilter/xt_sctp.c:60: error: for each function it appears in.) make[1]: *** [net/netfilter/xt_sctp.o] Error 1
make: *** [net/netfilter/xt_sctp.o] Error 2

Quoted 2 lines

The SCTP_CHUNKMAP_COPY uses "chunkmap" internally. What tree did you base this patch on?

Never mind, I figured out that both patches combined compile, so I've combined them in one. For the future: each patch should always compile on its own without errors.

author
#5 lore raw
Quoted 19 lines · Patrick McHardy
Quoted 18 lines · Patrick McHardy
Quoted 3 lines · lizf

Sorry for the noise, I've configed my mailer according to a kernel doc, but i'm still not sure whether it works. So here's the attachment.

Thanks, the attachment applies fine, but does not compile:

CC [M] net/netfilter/xt_sctp.o
net/netfilter/xt_sctp.c: In function 'match_packet':
net/netfilter/xt_sctp.c:60: error: 'chunkmap' undeclared (first use in this function)
net/netfilter/xt_sctp.c:60: error: (Each undeclared identifier is reported only once
net/netfilter/xt_sctp.c:60: error: for each function it appears in.) make[1]: *** [net/netfilter/xt_sctp.o] Error 1
make: *** [net/netfilter/xt_sctp.o] Error 2

Quoted 10 lines
Quoted 2 lines

The SCTP_CHUNKMAP_COPY uses "chunkmap" internally. What tree did you base this patch on?

Never mind, I figured out that both patches combined compile, so I've combined them in one. For the future: each patch should always compile on its own without errors.

I just thought out why it doesn't compile. I should have been more careful. Thanks for your patience.