lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 22 Jul 2010 18:16:48 +0300 From: Luciano Coelho <luciano.coelho@...ia.com> To: ext Jan Engelhardt <jengelh@...ozas.de> Cc: Netfilter Developer Mailing List <netfilter-devel@...r.kernel.org>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Patrick McHardy <kaber@...sh.net>, "sameo@...ux.intel.com" <sameo@...ux.intel.com>, Alexey Dobriyan <adobriyan@...il.com> Subject: Re: [RFC 1/1] netfilter: xtables: inclusion of xt_condition On Thu, 2010-07-22 at 16:44 +0200, ext Jan Engelhardt wrote: > On Thursday 2010-07-22 16:09, Luciano Coelho wrote: > >+static int condition_mt_check(const struct xt_mtchk_param *par) > >+{ > >+ struct xt_condition_mtinfo *info = par->matchinfo; > >+ struct condition_variable *var; > >+ struct condition_net *cond_net = > >+ condition_pernet(current->nsproxy->net_ns); > > Cc'ing Alexey who has done the netns support. > > Alexey, you added par->net, but given Luciano just did it with > current->nsproxy->net_ns, do we really need par->net? > > > >+int xt_condition_set_module_perms(const char *val, struct kernel_param *kp) > >+{ > >+ unsigned long l; > >+ int ret; > >+ struct condition_net *cond_net = > >+ condition_pernet(current->nsproxy->net_ns); > >+ > >+ if (!val) return -EINVAL; > > newline before return. Sure! I copied this from params.c. I'll fix it. > >+ ret = strict_strtoul(val, 0, &l); > >+ if (ret == -EINVAL || ((uint)l != l)) > >+ return -EINVAL; > > >+ *((u32 *) ((u8 *) cond_net + (size_t) kp->arg)) = l; > > I don't think we need this level of granularity; let the options be > global, similar to what xt_hashlimit does. I did this according to Patrick's comment: > > proc_net_condition is a global variable, so this won't work for > > namespaces. What the code does is reinitialize it when instantiating > > a new namespace, so it will always point to the last instantiated > > namespace. > > > > The same problem exists for the condition_list, each namespace > > should only be able to access its own conditions. > > This also applies to the permission variables. Basically, we shouldn't > be having any globals except perhaps the mutex. You probably need a > module_param_call function to set them for the correct namespace (you > can access that through current->nsproxy->net_ns). I found it a bit strange to be able to change the module params in a per-netns basis, but it is actually possible if you're changing the parameters via sysfs. I tried it and it even seems to work. ;) I can't see any module parameters in the xt_hashlimit.c file. Am I looking in the wrong place? I would be fine with making the module params global (as they were before), if that's fine with Patrick too. > (I am not even sure if kp->arg can be non-multiples-of-4, in which case > this would be an alignment violation even.) I'm passing size_t in kp->arg. It looks quite ugly, because usually kp->arg is a pointer to some data. But at least this way, using offsetof(), I could avoid lots of repeated code for the options... -- Cheers, Luca. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists