| 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, 23 Aug 2007 11:32:04 -0400
From: Brian Haley <brian.haley@...com>
To: Aurélien Charbon <aurelien.charbon@....bull.net>
Cc: Mailing list NFSv4 <nfsv4@...ux-nfs.org>,
netdev ML <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/1] NFS: change the ip_map cache code to handle IPv6
addresses
Hi Aurelien,
Aurélien Charbon wrote:
> According to Neil's comments, I have tried to correct the mistakes of my
> first sending
I have some more comments.
> @@ -1559,6 +1560,7 @@ exp_addclient(struct nfsctl_client *ncp)
> {
> struct auth_domain *dom;
> int i, err;
> + struct in6_addr addr6;
Indentation looks wrong.
> diff -p -u -r -N linux-2.6.23-rc3/fs/nfsd/nfsctl.c
> linux-2.6.23-rc3-IPv6-ipmap-cache/fs/nfsd/nfsctl.c
> --- linux-2.6.23-rc3/fs/nfsd/nfsctl.c 2007-08-23 13:18:16.000000000
> +0200
> +++ linux-2.6.23-rc3-IPv6-ipmap-cache/fs/nfsd/nfsctl.c 2007-08-23
> 13:25:28.000000000 +0200
> @@ -222,7 +222,7 @@ static ssize_t write_getfs(struct file *
> struct auth_domain *clp;
> int err = 0;
> struct knfsd_fh *res;
> -
> + struct in6_addr in6;
Indentation.
> if (size < sizeof(*data))
> return -EINVAL;
> data = (struct nfsctl_fsparm*)buf;
> @@ -236,7 +236,14 @@ static ssize_t write_getfs(struct file *
> res = (struct knfsd_fh*)buf;
>
> exp_readlock();
> - if (!(clp = auth_unix_lookup(sin->sin_addr)))
> +
> + /* IPv6 address mapping */
> + in6.s6_addr32[0] = 0;
> + in6.s6_addr32[1] = 0;
> + in6.s6_addr32[2] = htonl(0xffff);
> + in6.s6_addr32[3] = (uint32_t)sin->sin_addr.s_addr;
Why didn't you use your new ipv6_addr_map() inline here?
> @@ -253,6 +260,7 @@ static ssize_t write_getfd(struct file *
> {
> struct nfsctl_fdparm *data;
> struct sockaddr_in *sin;
> + struct in6_addr in6;
Indentation.
> @@ -271,7 +279,14 @@ static ssize_t write_getfd(struct file *
> res = buf;
> sin = (struct sockaddr_in *)&data->gd_addr;
> exp_readlock();
> - if (!(clp = auth_unix_lookup(sin->sin_addr)))
> +
> + /* IPv6 address mapping */
> + in6.s6_addr32[0] = 0;
> + in6.s6_addr32[1] = 0;
> + in6.s6_addr32[2] = htonl(0xffff);
> + in6.s6_addr32[3] = (uint32_t)sin->sin_addr.s_addr;
Why didn't you use your new ipv6_addr_map() inline here too?
> diff -p -u -r -N linux-2.6.23-rc3/include/net/ipv6.h
> linux-2.6.23-rc3-IPv6-ipmap-cache/include/net/ipv6.h
> --- linux-2.6.23-rc3/include/net/ipv6.h 2007-08-23 13:18:23.000000000
> +0200
> +++ linux-2.6.23-rc3-IPv6-ipmap-cache/include/net/ipv6.h 2007-08-23
> 13:25:28.000000000 +0200
> @@ -21,6 +21,7 @@
> #include <net/ndisc.h>
> #include <net/flow.h>
> #include <net/snmp.h>
> +#include <linux/in.h>
>
> #define SIN6_LEN_RFC2133 24
>
> @@ -167,6 +168,12 @@ DECLARE_SNMP_STAT(struct udp_mib, udplit
> if (is_udplite) SNMP_INC_STATS_USER(udplite_stats_in6,
> field); \
> else SNMP_INC_STATS_USER(udp_stats_in6, field); } while(0)
>
> +#define IS_ADDR_MAPPED(a) \
> + (((uint32_t *) (a))[0] == 0 \
> + && ((uint32_t *) (a))[1] == 0 \
> + && (((uint32_t *) (a))[2] == 0 \
> + || ((uint32_t *) (a))[2] == htonl(0xffff)))
I need to update a patch of mine that added a v4-mapped inline, let me
send that out. In the kernel you should use u32 too, is that why you
needed to include linux/net.h?
> +/* Maps a IPv4 address into a wright IPv6 address */
> +static inline int ipv6_addr_map(const struct in_addr a1, struct
> in6_addr a2)
> +{
> + a2.s6_addr32[0] = 0;
> + a2.s6_addr32[1] = 0;
> + a2.s6_addr32[2] = htonl(0xffff);
> + a2.s6_addr32[3] = (uint32_t)a1.s_addr;
> + return 0;
> +}
This can be void, noone ever checks the return status. Maybe change the
name to ipv6_addr_v4map() too?
> @@ -84,7 +85,7 @@ static void svcauth_unix_domain_release(
> struct ip_map {
> struct cache_head h;
> char m_class[8]; /* e.g. "nfsd" */
> - struct in_addr m_addr;
> + struct in6_addr m_addr;
Indentation.
> static void ip_map_init(struct cache_head *cnew, struct cache_head *citem)
> {
> @@ -125,7 +133,7 @@ static void ip_map_init(struct cache_hea
> struct ip_map *item = container_of(citem, struct ip_map, h);
>
> strcpy(new->m_class, item->m_class);
> - new->m_addr.s_addr = item->m_addr.s_addr;
> + memcpy(&(new->m_addr), &(item->m_addr), sizeof(struct in6_addr));
Use ipv6_addr_copy().
> @@ -151,20 +159,22 @@ static void ip_map_request(struct cache_
> {
> char text_addr[20];
> struct ip_map *im = container_of(h, struct ip_map, h);
> - __be32 addr = im->m_addr.s_addr;
> -
> - snprintf(text_addr, 20, "%u.%u.%u.%u",
> - ntohl(addr) >> 24 & 0xff,
> - ntohl(addr) >> 16 & 0xff,
> - ntohl(addr) >> 8 & 0xff,
> - ntohl(addr) >> 0 & 0xff);
>
> + if (IS_ADDR_MAPPED(im->m_addr.s6_addr32)) {
> + snprintf(text_addr, 20, NIPQUAD_FMT,
> + ntohl(im->m_addr.s6_addr32[3]) >> 24 & 0xff,
> + ntohl(im->m_addr.s6_addr32[3]) >> 16 & 0xff,
> + ntohl(im->m_addr.s6_addr32[3]) >> 8 & 0xff,
> + ntohl(im->m_addr.s6_addr32[3]) >> 0 & 0xff);
> + } else {
> + snprintf(text_addr, 20, NIP6_FMT, NIP6(im->m_addr));
> + }
You'll need more than 20 bytes to print an IPv6 address, I'd make this
at least 44 to account for some fluff. Surprised you didn't crash
during testing.
> static int ip_map_parse(struct cache_detail *cd,
> @@ -175,10 +185,10 @@ static int ip_map_parse(struct cache_det
> * for scratch: */
> char *buf = mesg;
> int len;
> - int b1,b2,b3,b4;
> + int b1, b2, b3, b4, b5, b6, b7, b8;
> char c;
> char class[8];
> - struct in_addr addr;
> + struct in6_addr addr;
> int err;
You might as well fix the indentation while you're here.
> @@ -238,7 +262,7 @@ static int ip_map_show(struct seq_file *
> struct cache_head *h)
> {
> struct ip_map *im;
> - struct in_addr addr;
> + struct in6_addr addr;
Indentation.
> if (h == NULL) {
> @@ -247,20 +271,33 @@ static int ip_map_show(struct seq_file *
> }
> im = container_of(h, struct ip_map, h);
> /* class addr domain */
> - addr = im->m_addr;
> + memcpy(&addr, &im->m_addr, sizeof(struct in6_addr));
ipv6_addr_copy()
> - seq_printf(m, "%s %d.%d.%d.%d %s\n",
> - im->m_class,
> - ntohl(addr.s_addr) >> 24 & 0xff,
> - ntohl(addr.s_addr) >> 16 & 0xff,
> - ntohl(addr.s_addr) >> 8 & 0xff,
> - ntohl(addr.s_addr) >> 0 & 0xff,
> - dom
> - );
> + if (IS_ADDR_MAPPED(addr.s6_addr32)) {
> + seq_printf(m, "%s" NIPQUAD_FMT "%s\n",
> + im->m_class,
> + ntohl(addr.s6_addr32[3]) >> 24 & 0xff,
> + ntohl(addr.s6_addr32[3]) >> 16 & 0xff,
> + ntohl(addr.s6_addr32[3]) >> 8 & 0xff,
> + ntohl(addr.s6_addr32[3]) >> 0 & 0xff,
> + dom);
> + } else {
> + seq_printf(m, "%s" NIP6_FMT "%s\n",
> + im->m_class,
> + ntohl(addr.s6_addr16[7]),
> + ntohl(addr.s6_addr16[6]),
> + ntohl(addr.s6_addr16[5]),
> + ntohl(addr.s6_addr16[4]),
> + ntohl(addr.s6_addr16[3]),
> + ntohl(addr.s6_addr16[2]),
> + ntohl(addr.s6_addr16[1]),
> + ntohl(addr.s6_addr16[0]),
> + dom);
These should be ntohs(), it's only a 16-bit quantity. Also, don't you
want to start printing this at addr[0], not addr[7]?
> -static struct ip_map *ip_map_lookup(char *class, struct in_addr addr)
> +static struct ip_map *ip_map_lookup(char *class, struct in6_addr addr)
> {
> struct ip_map ip;
> struct cache_head *ch;
>
> strcpy(ip.m_class, class);
> - ip.m_addr = addr;
> + memcpy(&ip.m_addr, &addr, sizeof(struct in6_addr));
ipv6_addr_copy()
-Brian
-
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