Fernando Fernandez Mancera <ffmancera@xxxxxxxxxx> wrote:
> +extern struct list_head nft_osf_fingers[2];
How is this going to be used?
I find it weird to see this in netfilter core.
> + f = nla_data(osf_attrs[OSF_ATTR_FINGER]);
> +
> + kf = kmalloc(sizeof(struct nf_osf_finger), GFP_KERNEL);
> + if (!kf)
> + return -ENOMEM;
> +
> + for (i = 0; i < ARRAY_SIZE(nft_osf_fingers); ++i)
> + INIT_LIST_HEAD(&nft_osf_fingers[i]);
> +
This is missing input validation.
I see no nla_policy for OSF_ATTR_FINGER.
Userspace could have placed anything from 0 to 0xffff bytes.
+ memcpy(&kf->finger, f, sizeof(struct nf_osf_user_finger));
Probably should use
nla_memdup() + an nla_plolicy struct entry.
Or nla_memdup() plus manual checking of nla_len() vs. expected/sane
values?
> + list_for_each_entry(sf, &nft_osf_fingers[!!f->df], finger_entry) {
> + if (memcmp(&sf->finger, f, sizeof(struct nf_osf_user_finger)))
> + continue;
> +
> + kfree(kf);
Hmm. So there can't be any duplicate entries in first place.
So I really wonder how this is going to be used or why
all of this code can't live in nft_osf.c .
I mean, we are adding this to core nftables api, and i think this is
something that should only be done if it can't be specific to particular
expression for some reason.
> + list_for_each_entry(sf, &nft_osf_fingers[!!f->df], finger_entry) {
> + if (memcmp(&sf->finger, f, sizeof(struct nf_osf_user_finger)))
> + continue;
list_for_each_entry_safe?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
