From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11515 invoked by alias); 15 Mar 2007 11:46:45 -0000 Received: (qmail 11499 invoked by uid 22791); 15 Mar 2007 11:46:45 -0000 X-Spam-Check-By: sourceware.org Received: from sunsite.ms.mff.cuni.cz (HELO sunsite.mff.cuni.cz) (195.113.15.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 15 Mar 2007 11:46:39 +0000 Received: from sunsite.mff.cuni.cz (localhost.localdomain [127.0.0.1]) by sunsite.mff.cuni.cz (8.13.8/8.13.8) with ESMTP id l2FBoPsA029521; Thu, 15 Mar 2007 12:50:25 +0100 Received: (from jakub@localhost) by sunsite.mff.cuni.cz (8.13.8/8.13.8/Submit) id l2FBoOwi029510; Thu, 15 Mar 2007 12:50:24 +0100 Date: Thu, 15 Mar 2007 11:46:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Glibc hackers Subject: [PATCH] Fix inet6_opt_{append,finish} Message-ID: <20070315115024.GM1826@sunsite.mff.cuni.cz> Reply-To: Jakub Jelinek Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.2i Mailing-List: contact libc-hacker-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sourceware.org X-SW-Source: 2007-03/txt/msg00021.txt.bz2 Hi! As the attached testcase shows, there have been several bugs: 1) if extbuf == NULL, extlen shouldn't be checked (happened both in inet6_opt_{append,finish}) 2) if the last option's end was 8 byte aligned already, inet6_opt_finish would say it needs 0 bytes to fill it, but then adding IP6OPT_PAD1 254 254x'\0' BTW, the npad calculations are too complicated: int npad = (align - data_offset % align) & (align - 1); is I believe the same as: int npad = -data_offset & (align - 1); and int npad = (8 - (offset & 7)) & 7; is int npad = -offset & 7; Apparently at least the latter isn't fully optimized on i?86 (but is on x86_64), guess ++todo for GCC. 2007-03-15 Jakub Jelinek [BZ #4181] * inet/inet6_opt.c (add_padding): Only insert padding if npad > 0. (inet6_opt_append): Don't check extlen is big enough if extbuf is NULL. (inet6_opt_finish): Likewise. * inet/Makefile (tests): Add test-inet6_opt. * inet/test-inet6_opt.c: New test. --- libc/inet/inet6_opt.c.jj 2006-05-25 06:38:02.000000000 +0200 +++ libc/inet/inet6_opt.c 2007-03-15 12:15:02.000000000 +0100 @@ -1,4 +1,4 @@ -/* Copyright (C) 2006 Free Software Foundation, Inc. +/* Copyright (C) 2006, 2007 Free Software Foundation, Inc. This file is part of the GNU C Library. Contributed by Ulrich Drepper , 2006. @@ -51,7 +51,7 @@ add_padding (uint8_t *extbuf, int offset { if (npad == 1) extbuf[offset] = IP6OPT_PAD1; - else + else if (npad > 0) { struct ip6_opt *pad_opt = (struct ip6_opt *) (extbuf + offset); @@ -102,21 +102,17 @@ inet6_opt_append (void *extbuf, socklen_ int data_offset = offset + sizeof (struct ip6_opt); int npad = (align - data_offset % align) & (align - 1); - /* Now we can check whether the buffer is large enough. */ - if (data_offset + npad + len > extlen) - return -1; - - if (npad != 0) + if (extbuf != NULL) { - if (extbuf != NULL) - add_padding (extbuf, offset, npad); + /* Now we can check whether the buffer is large enough. */ + if (data_offset + npad + len > extlen) + return -1; + + add_padding (extbuf, offset, npad); offset += npad; - } - /* Now prepare the option itself. */ - if (extbuf != NULL) - { + /* Now prepare the option itself. */ struct ip6_opt *opt = (struct ip6_opt *) ((uint8_t *) extbuf + offset); opt->ip6o_type = type; @@ -124,6 +120,8 @@ inet6_opt_append (void *extbuf, socklen_ *databufp = opt + 1; } + else + offset += npad; return offset + sizeof (struct ip6_opt) + len; } @@ -145,12 +143,14 @@ inet6_opt_finish (void *extbuf, socklen_ /* Required padding at the end. */ int npad = (8 - (offset & 7)) & 7; - /* Make sure the buffer is large enough. */ - if (offset + npad > extlen) - return -1; - if (extbuf != NULL) - add_padding (extbuf, offset, npad); + { + /* Make sure the buffer is large enough. */ + if (offset + npad > extlen) + return -1; + + add_padding (extbuf, offset, npad); + } return offset + npad; } --- libc/inet/Makefile.jj 2007-01-15 23:25:26.000000000 +0100 +++ libc/inet/Makefile 2007-03-15 12:26:53.000000000 +0100 @@ -52,7 +52,7 @@ routines := htonl htons \ aux := check_pf ifreq tests := htontest test_ifindex tst-ntoa tst-ether_aton tst-network \ - tst-gethnm test-ifaddrs bug-if1 + tst-gethnm test-ifaddrs bug-if1 test-inet6_opt include ../Rules --- libc/inet/test-inet6_opt.c.jj 2007-03-15 11:57:02.000000000 +0100 +++ libc/inet/test-inet6_opt.c 2007-03-15 12:29:21.000000000 +0100 @@ -0,0 +1,207 @@ +#include +#include +#include +#include + +#define OPT_X 42 +#define OPT_Y 43 +#define OPT_Z 44 + +static void * +encode_inet6_opt (socklen_t *elp) +{ + void *eb = NULL; + socklen_t el; + int cl; + void *db; + int offset; + uint8_t val1; + uint16_t val2; + uint32_t val4; + uint64_t val8; + + *elp = 0; +#define CHECK() \ + if (cl == -1) \ + { \ + printf ("cl == -1 on line %d\n", __LINE__); \ + free (eb); \ + return NULL; \ + } + + /* Estimate the length */ + cl = inet6_opt_init (NULL, 0); + CHECK (); + cl = inet6_opt_append (NULL, 0, cl, OPT_X, 12, 8, NULL); + CHECK (); + cl = inet6_opt_append (NULL, 0, cl, OPT_Y, 7, 4, NULL); + CHECK (); + cl = inet6_opt_append (NULL, 0, cl, OPT_Z, 7, 1, NULL); + CHECK (); + cl = inet6_opt_finish (NULL, 0, cl); + CHECK (); + el = cl; + + eb = malloc (el + 8); + if (eb == NULL) + { + puts ("malloc failed"); + return NULL; + } + /* Canary. */ + memcpy (eb + el, "deadbeef", 8); + + cl = inet6_opt_init (eb, el); + CHECK (); + + cl = inet6_opt_append (eb, el, cl, OPT_X, 12, 8, &db); + CHECK (); + val4 = 0x12345678; + offset = inet6_opt_set_val (db, 0, &val4, sizeof (val4)); + val8 = 0x0102030405060708LL; + inet6_opt_set_val (db, offset, &val8, sizeof (val8)); + + cl = inet6_opt_append (eb, el, cl, OPT_Y, 7, 4, &db); + CHECK (); + val1 = 0x01; + offset = inet6_opt_set_val (db, 0, &val1, sizeof (val1)); + val2 = 0x1331; + offset = inet6_opt_set_val (db, offset, &val2, sizeof (val2)); + val4 = 0x01020304; + inet6_opt_set_val (db, offset, &val4, sizeof (val4)); + + cl = inet6_opt_append (eb, el, cl, OPT_Z, 7, 1, &db); + CHECK (); + inet6_opt_set_val (db, 0, (void *) "abcdefg", 7); + + cl = inet6_opt_finish (eb, el, cl); + CHECK (); + + if (memcmp (eb + el, "deadbeef", 8) != 0) + { + puts ("Canary corrupted"); + free (eb); + return NULL; + } + *elp = el; + return eb; +} + +int +decode_inet6_opt (void *eb, socklen_t el) +{ + int ret = 0; + int seq = 0; + int cl = 0; + int offset; + uint8_t type; + socklen_t len; + uint8_t val1; + uint16_t val2; + uint32_t val4; + uint64_t val8; + void *db; + char buf[8]; + + while ((cl = inet6_opt_next (eb, el, cl, &type, &len, &db)) != -1) + switch (type) + { + case OPT_X: + if (seq++ != 0) + { + puts ("OPT_X is not first"); + ret = 1; + } + if (len != 12) + { + printf ("OPT_X's length %d != 12\n", len); + ret = 1; + } + offset = inet6_opt_get_val (db, 0, &val4, sizeof (val4)); + if (val4 != 0x12345678) + { + printf ("OPT_X's val4 %x != 0x12345678\n", val4); + ret = 1; + } + offset = inet6_opt_get_val (db, offset, &val8, sizeof (val8)); + if (offset != len || val8 != 0x0102030405060708LL) + { + printf ("OPT_X's val8 %llx != 0x0102030405060708\n", + (long long) val8); + ret = 1; + } + break; + case OPT_Y: + if (seq++ != 1) + { + puts ("OPT_Y is not second"); + ret = 1; + } + if (len != 7) + { + printf ("OPT_Y's length %d != 7\n", len); + ret = 1; + } + offset = inet6_opt_get_val (db, 0, &val1, sizeof (val1)); + if (val1 != 0x01) + { + printf ("OPT_Y's val1 %x != 0x01\n", val1); + ret = 1; + } + offset = inet6_opt_get_val (db, offset, &val2, sizeof (val2)); + if (val2 != 0x1331) + { + printf ("OPT_Y's val2 %x != 0x1331\n", val2); + ret = 1; + } + offset = inet6_opt_get_val (db, offset, &val4, sizeof (val4)); + if (offset != len || val4 != 0x01020304) + { + printf ("OPT_Y's val4 %x != 0x01020304\n", val4); + ret = 1; + } + break; + case OPT_Z: + if (seq++ != 2) + { + puts ("OPT_Z is not third"); + ret = 1; + } + if (len != 7) + { + printf ("OPT_Z's length %d != 7\n", len); + ret = 1; + } + offset = inet6_opt_get_val (db, 0, buf, 7); + if (offset != len || memcmp (buf, "abcdefg", 7) != 0) + { + buf[7] = '\0'; + printf ("OPT_Z's buf \"%s\" != \"abcdefg\"\n", buf); + ret = 1; + } + break; + default: + printf ("Unknown option %d\n", type); + ret = 1; + break; + } + if (seq != 3) + { + puts ("Didn't see all of OPT_X, OPT_Y and OPT_Z"); + ret = 1; + } + return ret; +} + +int +main (void) +{ + void *eb; + socklen_t el; + eb = encode_inet6_opt (&el); + if (eb == NULL) + return 1; + if (decode_inet6_opt (eb, el)) + return 1; + return 0; +} Jakub