From: Evgeniy Polyakov <zbr@ioremap•net>
To: Jan Engelhardt <jengelh@medozas•de>
Cc: Patrick McHardy <kaber@trash•net>,
netdev@vger•kernel.org, David Miller <davem@davemloft•net>,
"Paul E. McKenney" <paulmck@linux•vnet.ibm.com>,
Netfilter Development Mailinglist
<netfilter-devel@vger•kernel.org>
Subject: Re: Passive OS fingerprint xtables match (iptables part)
Date: Thu, 12 Feb 2009 22:18:24 +0300 [thread overview]
Message-ID: <20090212191824.GB17896@ioremap.net> (raw)
In-Reply-To: <alpine.LSU.2.00.0902121922280.20367@fbirervta.pbzchgretzou.qr>
On Thu, Feb 12, 2009 at 07:26:53PM +0100, Jan Engelhardt (jengelh@medozas•de) wrote:
> >#include <xtables.h>
> >
> >typedef unsigned int __u32;
> >typedef unsigned short __u16;
> >typedef unsigned char __u8;
>
> These should not done here.. it likely causes a "redefinition"
> warning or compile error of sorts. Include <linux/types.h> if
> in doubt.
It does not since linux/types.h header is not included, but better use
existing header of course.
> >static void osf_help(void)
> >{
> > printf("OS fingerprint match options:\n"
> > "--genre [!] string Match a OS genre by passive fingerprinting.\n"
>
> The syntax should be [!] --genre string, that is what most
> others use. Then the check_inverse call also be removed.
ok.
> >static const struct option osf_opts[] = {
> > { .name = "genre", .has_arg = 1, .flag = 0, .val = '1' },
> > { .name = "ttl", .has_arg = 1, .flag = 0, .val = '2' },
> > { .name = "log", .has_arg = 1, .flag = 0, .val = '3' },
> > { .name = "connector", .has_arg = 0, .flag = 0, .val = '5' },
> > { .name = NULL }
> >};
>
> .flag can be omitted; .has_arg=true
>
> > osf_parse_string(argv[optind-1], info);
> > if (invert)
> > info->flags |= IPT_OSF_INVERT;
> > info->len=strlen((char *)info->genre);
>
> This cast seems unnecessary.
Yup.
> > case '2': /* --ttl */
> > if (*flags & IPT_OSF_TTL)
> > exit_error(PARAMETER_PROBLEM, "Can't specify multiple ttl parameter");
> > *flags |= IPT_OSF_TTL;
> > info->flags |= IPT_OSF_TTL;
> > info->ttl = atoi(argv[optind-1]);
>
> Make use of xtables_strtoui to do bounds checking on the TTL value.
Hmm...
$ grep xtables_strtoui -r /tmp/iptables-1.4.2
$
> >static void osf_save(const void *ip, const struct xt_entry_match *match)
> >{
> > const struct ipt_osf_info *info = (const struct ipt_osf_info*) match->data;
> >
> > printf("--genre %s%s ", (info->flags & IPT_OSF_INVERT) ? "! ": "", info->genre);
> >}
>
> Similarly, put ! before.
ok.
> >static struct xtables_match osf_match = {
> > .name = "osf",
> > .version = XTABLES_VERSION,
> > .size = XT_ALIGN(sizeof(struct ipt_osf_info)),
> > .userspacesize = XT_ALIGN(sizeof(struct ipt_osf_info)),
> > .help = &osf_help,
> > .init = &osf_init,
> > .parse = &osf_parse,
> > .print = &osf_print,
> > .final_check = &osf_final_check,
> > .save = &osf_save,
> > .extra_opts = osf_opts
>
> The & for function pointers is not needed (and actually makes
> macro substituion break in some cases, just in case I need
> an excuse)
Well, having & clearly shows it is a pointer and not a value, especially
when it is not clear from the name what it should be.
--
Evgeniy Polyakov
next prev parent reply other threads:[~2009-02-12 19:18 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-12 17:12 Passive OS fingerprint xtables match Evgeniy Polyakov
2009-02-12 17:42 ` Paul E. McKenney
2009-02-12 17:51 ` Evgeniy Polyakov
2009-02-12 20:41 ` Paul E. McKenney
2009-02-18 14:55 ` Patrick McHardy
2009-02-12 18:22 ` Jan Engelhardt
2009-02-12 18:57 ` Evgeniy Polyakov
2009-02-12 20:12 ` Jan Engelhardt
2009-02-13 13:03 ` Evgeniy Polyakov
2009-02-13 13:51 ` Jan Engelhardt
2009-02-13 14:22 ` Evgeniy Polyakov
2009-02-13 14:41 ` Jan Engelhardt
2009-02-15 17:32 ` Evgeniy Polyakov
2009-02-18 15:02 ` Patrick McHardy
2009-02-18 15:07 ` Evgeniy Polyakov
2009-02-18 15:30 ` Jan Engelhardt
2009-02-19 11:56 ` Evgeniy Polyakov
2009-02-18 15:00 ` Patrick McHardy
2009-02-18 15:28 ` Jan Engelhardt
2009-02-12 18:26 ` Passive OS fingerprint xtables match (iptables part) Jan Engelhardt
2009-02-12 19:18 ` Evgeniy Polyakov [this message]
2009-02-12 20:19 ` Jan Engelhardt
2009-02-13 12:48 ` Evgeniy Polyakov
2009-02-13 12:52 ` Evgeniy Polyakov
2009-02-13 13:03 ` Jan Engelhardt
2009-02-13 13:12 ` Evgeniy Polyakov
2009-02-13 13:54 ` Jan Engelhardt
2009-02-15 17:35 ` Evgeniy Polyakov
2009-02-18 15:14 ` Passive OS fingerprint xtables match Patrick McHardy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090212191824.GB17896@ioremap.net \
--to=zbr@ioremap$(echo .)net \
--cc=davem@davemloft$(echo .)net \
--cc=jengelh@medozas$(echo .)de \
--cc=kaber@trash$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=netfilter-devel@vger$(echo .)kernel.org \
--cc=paulmck@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox