Re: dhclient(8): fix segfault if calloc()/strdup() return NULL
Am 11.12.2012 um 04:12 schrieb Lawrence Teo l...@openbsd.org:
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)
+ if (oldlease-server_name) {
newlease-server_name = strdup(oldlease-server_name);
- if (oldlease-filename)
+ if
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 m...@ii.net 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).
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.
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;
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;
else if (comp_codes_1g IXGBE_SFF_1GBASET_CAPABLE)
[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: [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.
