Re: [PATCH] OpenSSH: auth.c
On Thu, Dec 13, 2012 at 07:31:46PM +0100, Maxime Villard wrote: > Hi, > I was looking at some openssh code when I spotted a mistake applied, thanks. -- Darren Tucker (dtucker at zip.com.au) GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4 37C9 C982 80C7 8FF4 FA69 Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
[PATCH] OpenSSH: auth.c
Hi, I was looking at some openssh code when I spotted a mistake in a function from auth.c: static int secure_filename(FILE *f, const char *file, struct passwd *pw, char *err, size_t errlen) { char buf[MAXPATHLEN]; struct stat st; /* check the open file to avoid races */ if (fstat(fileno(f), &st) < 0) { snprintf(err, errlen, "cannot stat file %s: %s", buf, strerror(errno)); return -1; } return auth_secure_path(file, &st, pw->pw_dir, pw->pw_uid, err, errlen); } 'buf' is not initialized and used whereas it should be 'file'. Patch: --- auth.c 2012-12-08 12:51:32.0 +0100 +++ auth.c 2012-12-13 19:11:30.968193729 +0100 @@ -404,13 +404,12 @@ secure_filename(FILE *f, const char *file, struct passwd *pw, char *err, size_t errlen) { - char buf[MAXPATHLEN]; struct stat st; /* check the open file to avoid races */ if (fstat(fileno(f), &st) < 0) { snprintf(err, errlen, "cannot stat file %s: %s", - buf, strerror(errno)); + file, strerror(errno)); return -1; } return auth_secure_path(file, &st, pw->pw_dir, pw->pw_uid, err, errlen);
Re: ix: implement sfp+ module attach interrupt handling
On Thu, Dec 13, 2012 at 20:49 +0100, Mike Belopuhov wrote: > first this allows you to connect the sfp+ cable after boot > and correctly identify and use it, secondly it allows you > (supposedly as i've only tested two different types of > direct attach cables, i.e. different phys) to switch between > copper and fiber w/o rebooting. > > SDP1 interrupt processing is not enabled because i can't > test it. > > code from the intel driver (except for handling the "bad" hp > cable correctly -- and that's "b" not "a" for those who have > any idea what i'm talking about). > > OK? > a bit better arrangement of code + use change autoneg to use hw->phy.autoneg_advertised like FreeBSD does. in this case it should be set properly. forgot to mention that diff only affects 82599 and only when interface is up (since interrupts must be enabled). diff --git sys/dev/pci/if_ix.c sys/dev/pci/if_ix.c index 8c3f817..337f789 100644 --- sys/dev/pci/if_ix.c +++ sys/dev/pci/if_ix.c @@ -145,6 +145,8 @@ voidixgbe_setup_vlan_hw_support(struct ix_softc *); /* Support for pluggable optic modules */ intixgbe_sfp_probe(struct ix_softc *); void ixgbe_setup_optics(struct ix_softc *); +void ixgbe_handle_mod(struct ix_softc *); +void ixgbe_handle_msf(struct ix_softc *); /* Legacy (single vector interrupt handler */ intixgbe_legacy_irq(void *); @@ -928,12 +930,6 @@ ixgbe_legacy_irq(void *arg) return (0); } - if (ifp->if_flags & IFF_RUNNING) { - ixgbe_rxeof(que); - ixgbe_txeof(txr); - refill = 1; - } - /* Check for fan failure */ if ((hw->phy.media_type == ixgbe_media_type_copper) && (reg_eicr & IXGBE_EICR_GPI_SDP1)) { @@ -958,6 +954,28 @@ ixgbe_legacy_irq(void *arg) timeout_add_sec(&sc->timer, 1); } + if (hw->mac.type != ixgbe_mac_82598EB) { + if (reg_eicr & IXGBE_EICR_GPI_SDP2) { + /* Clear the interrupt */ + IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP2); + ixgbe_handle_mod(sc); + } +#if 0 + else if ((hw->phy.media_type != ixgbe_media_type_copper) && + (reg_eicr & IXGBE_EICR_GPI_SDP1)) { + /* Clear the interrupt */ + IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP1); + ixgbe_handle_msf(sc); + } +#endif + } + + if (ifp->if_flags & IFF_RUNNING) { + ixgbe_rxeof(que); + ixgbe_txeof(txr); + refill = 1; + } + if (refill) { if (ixgbe_rxfill(que->rxr)) { /* Advance the Rx Queue "Tail Pointer" */ @@ -3408,6 +3426,50 @@ out: return (result); } +/* + * SFP module interrupts handler + */ +void +ixgbe_handle_mod(struct ix_softc *sc) +{ + struct ixgbe_hw *hw = &sc->hw; + uint32_t err; + + err = hw->phy.ops.identify_sfp(hw); + if (err == IXGBE_ERR_SFP_NOT_SUPPORTED) { + printf("%s: Unsupported SFP+ module type was detected!\n", + sc->dev.dv_xname); + return; + } + err = hw->mac.ops.setup_sfp(hw); + if (err == IXGBE_ERR_SFP_NOT_SUPPORTED) { + printf("%s: Setup failure - unsupported SFP+ module type!\n", + sc->dev.dv_xname); + return; + } + /* Set the optics type so system reports correctly */ + ixgbe_setup_optics(sc); + return (ixgbe_handle_msf(sc)); +} + + +/* + * MSF (multispeed fiber) interrupts handler + */ +void +ixgbe_handle_msf(struct ix_softc *sc) +{ + struct ixgbe_hw *hw = &sc->hw; + uint32_t autoneg; + int negotiate; + + autoneg = hw->phy.autoneg_advertised; + if ((!autoneg) && (hw->mac.ops.get_link_capabilities)) + hw->mac.ops.get_link_capabilities(hw, &autoneg, &negotiate); + if (hw->mac.ops.setup_link) + hw->mac.ops.setup_link(hw, autoneg, negotiate, TRUE); +} + /** * * Update the board statistics counters. diff --git sys/dev/pci/ixgbe_82599.c sys/dev/pci/ixgbe_82599.c index 350ddc7..e91579d 100644 --- sys/dev/pci/ixgbe_82599.c +++ sys/dev/pci/ixgbe_82599.c @@ -1350,6 +1350,11 @@ sfp_check: physical_layer = IXGBE_PHYSICAL_LAYER_10GBASE_SR; else if (comp_codes_10g & IXGBE_SFF_10GBASELR_CAPABLE) physical_layer = IXGBE_PHYSICAL_LAYER_10GBASE_LR; + else if (comp_codes_10g & + (IXGBE_SFF_DA_PASSIVE_CABLE | IXGBE_SFF_DA_BAD_HP_CABLE)) + physical_layer = IXGBE_PHYSICAL_LAYER_SFP_PLUS_CU; + else if (comp_codes_10g & IXGBE_SFF_DA_ACTIVE_CABLE) + physical_layer = IXGBE_PHYSICAL_LAYER_SFP_ACTIVE_DA;
ix: implement sfp+ module attach interrupt handling
first this allows you to connect the sfp+ cable after boot and correctly identify and use it, secondly it allows you (supposedly as i've only tested two different types of direct attach cables, i.e. different phys) to switch between copper and fiber w/o rebooting. SDP1 interrupt processing is not enabled because i can't test it. code from the intel driver (except for handling the "bad" hp cable correctly -- and that's "b" not "a" for those who have any idea what i'm talking about). OK? diff --git sys/dev/pci/if_ix.c sys/dev/pci/if_ix.c index 8c3f817..414d54f 100644 --- sys/dev/pci/if_ix.c +++ sys/dev/pci/if_ix.c @@ -145,6 +145,8 @@ voidixgbe_setup_vlan_hw_support(struct ix_softc *); /* Support for pluggable optic modules */ intixgbe_sfp_probe(struct ix_softc *); void ixgbe_setup_optics(struct ix_softc *); +void ixgbe_handle_mod(struct ix_softc *); +void ixgbe_handle_msf(struct ix_softc *); /* Legacy (single vector interrupt handler */ intixgbe_legacy_irq(void *); @@ -928,12 +930,6 @@ ixgbe_legacy_irq(void *arg) return (0); } - if (ifp->if_flags & IFF_RUNNING) { - ixgbe_rxeof(que); - ixgbe_txeof(txr); - refill = 1; - } - /* Check for fan failure */ if ((hw->phy.media_type == ixgbe_media_type_copper) && (reg_eicr & IXGBE_EICR_GPI_SDP1)) { @@ -958,6 +954,28 @@ ixgbe_legacy_irq(void *arg) timeout_add_sec(&sc->timer, 1); } + if (hw->mac.type != ixgbe_mac_82598EB) { + if (reg_eicr & IXGBE_EICR_GPI_SDP2) { + /* Clear the interrupt */ + IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP2); + ixgbe_handle_mod(sc); + } +#if 0 + else if ((hw->phy.media_type != ixgbe_media_type_copper) && + (reg_eicr & IXGBE_EICR_GPI_SDP1)) { + /* Clear the interrupt */ + IXGBE_WRITE_REG(hw, IXGBE_EICR, IXGBE_EICR_GPI_SDP1); + ixgbe_handle_msf(sc); + } +#endif + } + + if (ifp->if_flags & IFF_RUNNING) { + ixgbe_rxeof(que); + ixgbe_txeof(txr); + refill = 1; + } + if (refill) { if (ixgbe_rxfill(que->rxr)) { /* Advance the Rx Queue "Tail Pointer" */ @@ -3408,6 +3426,53 @@ out: return (result); } +/* + * SFP module interrupts handler + */ +void +ixgbe_handle_mod(struct ix_softc *sc) +{ + struct ixgbe_hw *hw = &sc->hw; + uint32_t err; + + err = hw->phy.ops.identify_sfp(hw); + if (err == IXGBE_ERR_SFP_NOT_SUPPORTED) { + printf("%s: Unsupported SFP+ module type was detected!\n", + sc->dev.dv_xname); + return; + } + err = hw->mac.ops.setup_sfp(hw); + if (err == IXGBE_ERR_SFP_NOT_SUPPORTED) { + printf("%s: Setup failure - unsupported SFP+ module type!\n", + sc->dev.dv_xname); + return; + } + return (ixgbe_handle_msf(sc)); +} + + +/* + * MSF (multispeed fiber) interrupts handler + */ +void +ixgbe_handle_msf(struct ix_softc *sc) +{ + struct ixgbe_hw *hw = &sc->hw; + uint32_t autoneg; + int negotiate; + + /* XXX: must be changeable in ixgbe_media_change */ + autoneg = IXGBE_LINK_SPEED_100_FULL | + IXGBE_LINK_SPEED_1GB_FULL | + IXGBE_LINK_SPEED_10GB_FULL; + if ((!autoneg) && (hw->mac.ops.get_link_capabilities)) + hw->mac.ops.get_link_capabilities(hw, &autoneg, &negotiate); + if (hw->mac.ops.setup_link) + hw->mac.ops.setup_link(hw, autoneg, negotiate, TRUE); + /* Set the optics type so system reports correctly */ + ixgbe_setup_optics(sc); +} + /** * * Update the board statistics counters. diff --git sys/dev/pci/ixgbe_82599.c sys/dev/pci/ixgbe_82599.c index 350ddc7..e91579d 100644 --- sys/dev/pci/ixgbe_82599.c +++ sys/dev/pci/ixgbe_82599.c @@ -1350,6 +1350,11 @@ sfp_check: physical_layer = IXGBE_PHYSICAL_LAYER_10GBASE_SR; else if (comp_codes_10g & IXGBE_SFF_10GBASELR_CAPABLE) physical_layer = IXGBE_PHYSICAL_LAYER_10GBASE_LR; + else if (comp_codes_10g & + (IXGBE_SFF_DA_PASSIVE_CABLE | IXGBE_SFF_DA_BAD_HP_CABLE)) + physical_layer = IXGBE_PHYSICAL_LAYER_SFP_PLUS_CU; + else if (comp_codes_10g & IXGBE_SFF_DA_ACTIVE_CABLE) + physical_layer = IXGBE_PHYSICAL_LAYER_SFP_ACTIVE_DA; else if (comp_codes_1g & IXGBE_SFF_1GBASET_CAPABLE) physical_layer = IXGBE_PHYSICAL_LAYER_1000BASE_T; break;
ospf6d: redistribute blackholed default
kroute.c rev 1.69 of ospfd for ospf6d date: 2009/06/02 20:16:59; author: claudio; state: Exp; lines: +13 -3 Track reject and blackhole routes and allow them to be redistributed even though they point to the loopback. Mainly used for redistribute default since on default free routers we need to have a fake route now. After discussion with Tonnerre Lombard, idea OK henning@ ok? Index: kroute.c === RCS file: /cvs/src/usr.sbin/ospf6d/kroute.c,v retrieving revision 1.40 diff -u -p -r1.40 kroute.c --- kroute.c21 Oct 2012 21:30:44 - 1.40 +++ kroute.c13 Dec 2012 16:14:12 - @@ -393,9 +393,11 @@ kr_redist_eval(struct kroute *kr, struct IN6_IS_ADDR_V4COMPAT(&kr->prefix)) goto dont_redistribute; /* -* Consider networks with nexthop loopback as not redistributable. +* Consider networks with nexthop loopback as not redistributable +* unless it is a reject or blackhole route. */ - if (IN6_IS_ADDR_LOOPBACK(&kr->nexthop)) + if (IN6_IS_ADDR_LOOPBACK(&kr->nexthop) && + !(kr->flags & (F_BLACKHOLE|F_REJECT))) goto dont_redistribute; /* Should we redistrubute this route? */ @@ -1106,6 +1108,10 @@ fetchtable(void) sa_in6 = (struct sockaddr_in6 *)rti_info[RTAX_NETMASK]; if (rtm->rtm_flags & RTF_STATIC) kr->r.flags |= F_STATIC; + if (rtm->rtm_flags & RTF_BLACKHOLE) + kr->r.flags |= F_BLACKHOLE; + if (rtm->rtm_flags & RTF_REJECT) + kr->r.flags |= F_REJECT; if (rtm->rtm_flags & RTF_DYNAMIC) kr->r.flags |= F_DYNAMIC; if (rtm->rtm_flags & RTF_PROTO1) @@ -1308,6 +1314,10 @@ dispatch_rtmsg(void) fatalx("classful IPv6 address?!!"); if (rtm->rtm_flags & RTF_STATIC) flags |= F_STATIC; + if (rtm->rtm_flags & RTF_BLACKHOLE) + flags |= F_BLACKHOLE; + if (rtm->rtm_flags & RTF_REJECT) + flags |= F_REJECT; if (rtm->rtm_flags & RTF_DYNAMIC) flags |= F_DYNAMIC; if (rtm->rtm_flags & RTF_PROTO1) Index: ospf6d.h === RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.h,v retrieving revision 1.25 diff -u -p -r1.25 ospf6d.h --- ospf6d.h22 Oct 2012 07:28:49 - 1.25 +++ ospf6d.h13 Dec 2012 16:14:12 - @@ -56,7 +56,9 @@ #defineF_DOWN 0x0010 #defineF_STATIC0x0020 #defineF_DYNAMIC 0x0040 -#defineF_REDISTRIBUTED 0x0100 +#defineF_REJECT0x0080 +#defineF_BLACKHOLE 0x0100 +#defineF_REDISTRIBUTED 0x0200 struct imsgev { struct imsgbuf ibuf; -- I'm not entirely sure you are real.
Re: [PATCH] remove useless casts in if_gre.c
On 03:07 Thu 13 Dec , Rafael Ferreira Neves wrote: > Hello, > > I think that the original code is correct. The definition of ip_output > function says that the first argument is a pointer to an mbuf, but the > other are variadic. In this case style(9) states that (type *)NULL should > be used. > > Regards > Rafael > > On Thu, Dec 13, 2012 at 2:11 AM, Michael W. Bombardieri wrote: > > > Hi, > > > > I have a small patch for if_gre.c... > > > > NULL is already defined as ((void *)0), so we don't > > need to cast it to void*. > > > > No binary change on amd64. > > Does this look OK? > > > > - Michael > > > > > > Index: if_gre.c > > === > > RCS file: /cvs/src/sys/net/if_gre.c,v > > retrieving revision 1.59 > > diff -u -r1.59 if_gre.c > > --- if_gre.c23 Nov 2012 20:12:03 - 1.59 > > +++ if_gre.c13 Dec 2012 04:09:38 - > > @@ -431,7 +431,7 @@ > > #endif > > > > /* Send it off */ > > - error = ip_output(m, (void *)NULL, &sc->route, 0, (void *)NULL, > > (void *)NULL); > > + error = ip_output(m, NULL, &sc->route, 0, NULL, NULL); > >end: > > if (error) > > ifp->if_oerrors++; And after cpp run you get a double cast like (void*)((void*)0).
Re: dhclient(8): fix segfault if calloc()/strdup() return NULL
Am 11.12.2012 um 04:12 schrieb Lawrence Teo : > There are a number of calloc() and strdup() calls in the > apply_defaults() and clone_lease() functions whose return values are not > checked. If they happen to return NULL, dhclient will segfault. > > This diff checks their return values and does some cleanup if they > return NULL. The diff also ensures that dhclient will not attempt to > make any changes (e.g. delete addresses or flush routes) if it > encounters these errors, but instead will try again at the next retry > interval. > > Thoughts/OK? Instead of checking each and every result: What about just introducing xstrdup() and xcalloc() similar to xmalloc.c in SSH? Or is recovering from OOM case considered important here? > Index: dhclient.c > === > RCS file: /cvs/src/sbin/dhclient/dhclient.c,v > retrieving revision 1.191 > diff -u -p -r1.191 dhclient.c > --- dhclient.c9 Dec 2012 20:28:03 - 1.191 > +++ dhclient.c10 Dec 2012 04:09:12 - > @@ -677,10 +677,17 @@ bind_lease(void) > struct client_lease *lease; > char *domainname, *nameservers; > > + lease = apply_defaults(client->new); > + if (lease == NULL) { > + client->state = S_INIT; > + set_timeout_interval(config->retry_interval, state_init); > + go_daemon(); > + return; > + } > + > delete_addresses(ifi->name, ifi->rdomain); > flush_routes_and_arp_cache(ifi->rdomain); > > - lease = apply_defaults(client->new); > options = lease->options; > > memset(&mask, 0, sizeof(mask)); > @@ -1939,6 +1946,10 @@ apply_defaults(struct client_lease *leas > int i, j; > > newlease = clone_lease(lease); > + if (newlease == NULL) { > + warning("Unable to clone lease"); > + return (NULL); > + } > > for (i = 0; i < 256; i++) { > for (j = 0; j < config->ignored_option_count; j++) { > @@ -1959,6 +1970,8 @@ apply_defaults(struct client_lease *leas > newlease->options[i].len = config->defaults[i].len; > newlease->options[i].data = calloc(1, > config->defaults[i].len); > + if (newlease->options[i].data == NULL) > + goto cleanup; > memcpy(newlease->options[i].data, > config->defaults[i].data, config->defaults[i].len); > break; > @@ -1970,6 +1983,8 @@ apply_defaults(struct client_lease *leas > lease->options[i].len; > newlease->options[i].data = calloc(1, > newlease->options[i].len); > + if (newlease->options[i].data == NULL) > + goto cleanup; > memcpy(newlease->options[i].data, > config->defaults[i].data, config->defaults[i].len); > memcpy(newlease->options[i].data + > @@ -1984,6 +1999,8 @@ apply_defaults(struct client_lease *leas > lease->options[i].len; > newlease->options[i].data = calloc(1, > newlease->options[i].len); > + if (newlease->options[i].data == NULL) > + goto cleanup; > memcpy(newlease->options[i].data, > lease->options[i].data, lease->options[i].len); > memcpy(newlease->options[i].data + > @@ -1998,6 +2015,8 @@ apply_defaults(struct client_lease *leas > config->defaults[i].len; > newlease->options[i].data = calloc(1, > config->defaults[i].len); > + if (newlease->options[i].data == NULL) > + goto cleanup; > memcpy(newlease->options[i].data, > config->defaults[i].data, > config->defaults[i].len); > @@ -2010,6 +2029,14 @@ apply_defaults(struct client_lease *leas > } > > return (newlease); > + > +cleanup: > + if (newlease) > + free_client_lease(newlease); > + > + warning("Unable to apply defaults"); > + > + return (NULL); > } > > struct client_lease * > @@ -2019,6 +2046,8 @@ clone_lease(struct client_lease *oldleas > int i; > > newlease = calloc(1, sizeof(struct client_lease)); > + if (newlease == NULL) > + goto cleanup; > > newlease->expiry = oldlease->expiry; > newlease->renewal = oldlease->renewal; > @@ -2026,19 +2055,33 @@ clone_lease(struct client_lease *oldleas > newlease->is_static = oldlease->is_static; > newlease->is_bootp = oldlease->is_bootp; > > - if (oldlease->server_name) >