Return a rtentry* in arplookup()
This brings arplookup() in sync with nd6_lookup() and is necessary for
upcoming struct rtentry reference count.
Ok?
Index: netinet/if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.160
diff -u -p -r1.160 if_ether.c
--- netinet/if_ether.c 17 Aug 2015 09:58:10 - 1.160
+++ netinet/if_ether.c 17 Aug 2015 10:02:22 -
@@ -93,7 +93,7 @@ int arpt_down = 20; /* once declared do
void arptfree(struct llinfo_arp *);
void arptimer(void *);
-struct llinfo_arp *arplookup(u_int32_t, int, int, u_int);
+struct rtentry *arplookup(u_int32_t, int, int, u_int);
void in_arpinput(struct mbuf *);
LIST_HEAD(, llinfo_arp) llinfo_arp;
@@ -372,9 +372,9 @@ arpresolve(struct ifnet *ifp, struct rte
local address\n, __func__, inet_ntop(AF_INET,
satosin(dst)-sin_addr, addr, sizeof(addr)));
} else {
- if ((la = arplookup(satosin(dst)-sin_addr.s_addr, 1, 0,
+ if ((rt = arplookup(satosin(dst)-sin_addr.s_addr, 1, 0,
ifp-if_rdomain)) != NULL)
- rt = la-la_rt;
+ la = ((struct llinfo_arp *)rt-rt_llinfo);
else
log(LOG_DEBUG, %s: %s: can't allocate llinfo\n,
__func__,
@@ -633,9 +633,10 @@ in_arpinput(struct mbuf *m)
itaddr = myaddr;
goto reply;
}
- la = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0,
+ rt = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0,
rtable_l2(m-m_pkthdr.ph_rtableid));
- if (la (rt = la-la_rt) (sdl = SDL(rt-rt_gateway))) {
+ if (rt != NULL (la = (struct llinfo_arp *)rt-rt_llinfo)
+ (sdl = SDL(rt-rt_gateway))) {
if (sdl-sdl_alen) {
if (memcmp(ea-arp_sha, LLADDR(sdl), sdl-sdl_alen)) {
if (rt-rt_flags
@@ -730,11 +731,10 @@ out:
memcpy(ea-arp_tha, ea-arp_sha, sizeof(ea-arp_sha));
memcpy(ea-arp_sha, enaddr, sizeof(ea-arp_sha));
} else {
- la = arplookup(itaddr.s_addr, 0, SIN_PROXY,
+ rt = arplookup(itaddr.s_addr, 0, SIN_PROXY,
rtable_l2(m-m_pkthdr.ph_rtableid));
- if (la == NULL)
+ if (rt == NULL)
goto out;
- rt = la-la_rt;
if (rt-rt_ifp-if_type == IFT_CARP ifp-if_type != IFT_CARP)
goto out;
memcpy(ea-arp_tha, ea-arp_sha, sizeof(ea-arp_sha));
@@ -790,7 +790,7 @@ arptfree(struct llinfo_arp *la)
/*
* Lookup or enter a new address in arptab.
*/
-struct llinfo_arp *
+struct rtentry *
arplookup(u_int32_t addr, int create, int proxy, u_int tableid)
{
struct rtentry *rt;
@@ -818,7 +818,7 @@ arplookup(u_int32_t addr, int create, in
}
return (0);
}
- return ((struct llinfo_arp *)rt-rt_llinfo);
+ return (rt);
}
/*
@@ -827,13 +827,15 @@ arplookup(u_int32_t addr, int create, in
int
arpproxy(struct in_addr in, u_int rdomain)
{
+ struct rtentry *rt;
struct llinfo_arp *la;
struct ifnet *ifp;
int found = 0;
- la = arplookup(in.s_addr, 0, SIN_PROXY, rdomain);
- if (la == NULL)
+ rt = arplookup(in.s_addr, 0, SIN_PROXY, rdomain);
+ if (rt == NULL)
return (0);
+ la = ((struct llinfo_arp *)rt-rt_llinfo);
TAILQ_FOREACH(ifp, ifnet, if_list) {
if (ifp-if_rdomain != rdomain)
rtfree(9) and nd6_prefix_{on,off}link
Ultimately my goal is to use rt_ifa_{add,del}() instead of
nd6_prefix_{on,off}link() but right now I need to remove the
rt-ref_cnt--.
This diff does that and reduce the differences between these functions.
Note that nd6_prefix_onlink()'s error are always logged so I doubt we
need two messages.
Ok?
Index: netinet6/nd6.h
===
RCS file: /cvs/src/sys/netinet6/nd6.h,v
retrieving revision 1.44
diff -u -p -r1.44 nd6.h
--- netinet6/nd6.h 18 Jul 2015 15:05:32 - 1.44
+++ netinet6/nd6.h 17 Aug 2015 10:04:04 -
@@ -309,8 +309,6 @@ void prelist_remove(struct nd_prefix *);
int prelist_update(struct nd_prefix *, struct nd_defrouter *, struct mbuf *);
int nd6_prelist_add(struct nd_prefix *, struct nd_defrouter *,
struct nd_prefix **);
-int nd6_prefix_onlink(struct nd_prefix *);
-int nd6_prefix_offlink(struct nd_prefix *);
void pfxlist_onlink_check(void);
struct nd_defrouter *defrouter_lookup(struct in6_addr *, struct ifnet *);
Index: netinet6/nd6_rtr.c
===
RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
retrieving revision 1.113
diff -u -p -r1.113 nd6_rtr.c
--- netinet6/nd6_rtr.c 18 Jul 2015 15:51:17 - 1.113
+++ netinet6/nd6_rtr.c 17 Aug 2015 10:04:05 -
@@ -69,7 +69,8 @@ void pfxrtr_del(struct nd_pfxrouter *);
struct nd_pfxrouter *find_pfxlist_reachable_router(struct nd_prefix *);
void defrouter_delreq(struct nd_defrouter *);
void purge_detached(struct ifnet *);
-
+int nd6_prefix_onlink(struct nd_prefix *);
+int nd6_prefix_offlink(struct nd_prefix *);
void in6_init_address_ltimes(struct nd_prefix *, struct in6_addrlifetime *);
int rt6_deleteroute(struct rtentry *, void *, unsigned int);
@@ -601,7 +602,7 @@ defrouter_addreq(struct nd_defrouter *ne
{
struct rt_addrinfo info;
struct sockaddr_in6 def, mask, gate;
- struct rtentry *newrt = NULL;
+ struct rtentry *rt = NULL;
int s;
int error;
@@ -622,11 +623,11 @@ defrouter_addreq(struct nd_defrouter *ne
info.rti_info[RTAX_NETMASK] = sin6tosa(mask);
s = splsoftnet();
- error = rtrequest1(RTM_ADD, info, RTP_DEFAULT, newrt,
+ error = rtrequest1(RTM_ADD, info, RTP_DEFAULT, rt,
new-ifp-if_rdomain);
- if (newrt) {
- rt_sendmsg(newrt, RTM_ADD, new-ifp-if_rdomain);
- newrt-rt_refcnt--;
+ if (rt) {
+ rt_sendmsg(rt, RTM_ADD, new-ifp-if_rdomain);
+ rtfree(rt);
}
if (error == 0)
new-installed = 1;
@@ -1783,14 +1784,8 @@ nd6_prefix_onlink(struct nd_prefix *pr)
char addr[INET6_ADDRSTRLEN];
/* sanity check */
- if ((pr-ndpr_stateflags NDPRF_ONLINK) != 0) {
- nd6log((LOG_ERR,
- nd6_prefix_onlink: %s/%d is already on-link\n,
- inet_ntop(AF_INET6, pr-ndpr_prefix.sin6_addr,
- addr, sizeof(addr)),
- pr-ndpr_plen));
+ if ((pr-ndpr_stateflags NDPRF_ONLINK) != 0)
return (EEXIST);
- }
/*
* Add the interface route associated with the prefix. Before
@@ -1861,26 +1856,11 @@ nd6_prefix_onlink(struct nd_prefix *pr)
info.rti_info[RTAX_NETMASK] = sin6tosa(mask6);
error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, rt, ifp-if_rdomain);
- if (error == 0) {
- if (rt != NULL) /* this should be non NULL, though */
- rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
+ if (error == 0 rt != NULL) {
pr-ndpr_stateflags |= NDPRF_ONLINK;
- } else {
- char gw[INET6_ADDRSTRLEN], mask[INET6_ADDRSTRLEN];
- nd6log((LOG_ERR, nd6_prefix_onlink: failed to add route for a
-prefix (%s/%d) on %s, gw=%s, mask=%s, flags=%lx
- errno = %d\n,
- inet_ntop(AF_INET6, pr-ndpr_prefix.sin6_addr,
- addr, sizeof(addr)),
- pr-ndpr_plen, ifp-if_xname,
- inet_ntop(AF_INET6, satosin6(ifa-ifa_addr)-sin6_addr,
- gw, sizeof(gw)),
- inet_ntop(AF_INET6, mask6.sin6_addr, mask, sizeof(mask)),
- rtflags, error));
+ rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
+ rtfree(rt);
}
-
- if (rt != NULL)
- rt-rt_refcnt--;
return (error);
}
Re: Don't be verbose in in6_update_ifa()
On 11/08/15(Tue) 16:25, Alexander Bluhm wrote:
On Mon, Aug 10, 2015 at 10:43:46PM +0200, Martin Pieuchot wrote:
In general these messages do not help. Here we have two cases of
messages, either logged at LOG_INFO or LOG_ERR.
Yes, multiple logs for the same error are bad. But there is a path
where we ignore the return code of in6_update_ifa().
in6_update_ifa()
in6_ifattach_loopback()
in6_ifattach()
The latter is called without error check from
if_up()
ifioctl()
ifnewlladdr()
in6_control()
So we should either propagate the error in these functions or add
a log somewhere in the call stack.
What about committing this diff first?
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.359
diff -u -p -r1.359 if.c
--- net/if.c16 Aug 2015 12:19:06 - 1.359
+++ net/if.c17 Aug 2015 11:04:39 -
@@ -1314,11 +1314,11 @@ ifioctl(struct socket *so, u_long cmd, c
case AF_INET6:
s = splsoftnet();
if (cmd == SIOCIFAFATTACH)
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
else
in6_ifdetach(ifp);
splx(s);
- return (0);
+ return (error);
#endif /* INET6 */
default:
return (EAFNOSUPPORT);
@@ -1382,8 +1382,10 @@ ifioctl(struct socket *so, u_long cmd, c
#ifdef INET6
if (ISSET(ifr-ifr_flags, IFXF_AUTOCONF6)) {
s = splsoftnet();
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
splx(s);
+ if (error != 0)
+ return (error);
}
if ((ifr-ifr_flags IFXF_AUTOCONF6)
Index: netinet6//in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.162
diff -u -p -r1.162 in6.c
--- netinet6//in6.c 12 Aug 2015 09:06:18 - 1.162
+++ netinet6//in6.c 17 Aug 2015 11:00:04 -
@@ -479,7 +479,11 @@ in6_control(struct socket *so, u_long cm
* is no link-local yet.
*/
s = splsoftnet();
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
+ if (error != 0) {
+ splx(s);
+ return (error);
+ }
error = in6_update_ifa(ifp, ifra, ia6);
splx(s);
if (error != 0)
Re: [Patch] pf refactoring
Hello Richard, On 17/08/15(Mon) 17:39, Richard Procter wrote: Hi, This series of 29 small diffs slims pf.o by 2640 bytes and pf.c by 113 non-comment lines. We generally discuss one diff per mail. It makes it easier for people to comment and as you can imagine deal with possible conflicts in their tree :) As you might now, Sasha has some upcoming changes with pending review :) pf_translate(), in particular, is now much shorter and clearer[0]. [...] [0] As pf_translate() is called only on state creation it isn't terribly performance sensitive, either. This is a valid point, out of curiosity what drove you in this part of the code? Martin
rtfree(9) conversion in rt_getifa()
Here a route lookup is done to find an existing ``ifa'' in order to
attach a new route.
This code runs under KERNEL_LOCK and returns a route entry found in
the table (RT_VALID), so rtfree(9) will not free the route.
Note that it is safe to rtfree() a route entry while keeping a pointer
to a *valid* ``ifa'' because ifa insertion/removal are serialized by
the KERNEL_LOCK. Yes, route entries increment ifas refcounter but this
only matters for stall ifas, and there's already a check for that here.
Ok?
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.220
diff -u -p -r1.220 route.c
--- net/route.c 17 Aug 2015 09:50:12 - 1.220
+++ net/route.c 17 Aug 2015 10:25:24 -
@@ -666,14 +666,18 @@ ifa_ifwithroute(int flags, struct sockad
struct rtentry *rt = rtalloc(gateway, 0, rtableid);
if (rt == NULL)
return (NULL);
- rt-rt_refcnt--;
/* The gateway must be local if the same address family. */
if ((rt-rt_flags RTF_GATEWAY)
- rt_key(rt)-sa_family == dst-sa_family)
+ rt_key(rt)-sa_family == dst-sa_family) {
+ rtfree(rt);
return (NULL);
+ }
ifa = rt-rt_ifa;
- if (ifa == NULL || ifa-ifa_ifp == NULL)
+ if (ifa == NULL || ifa-ifa_ifp == NULL) {
+ rtfree(rt);
return (NULL);
+ }
+ rtfree(rt);
}
if (ifa-ifa_addr-sa_family != dst-sa_family) {
struct ifaddr *oifa = ifa;
Re: [Patch] pf refactoring
On 17/08/15(Mon) 13:38, Alexey Suslikov wrote: Martin Pieuchot mpi at openbsd.org writes: On 17/08/15(Mon) 17:39, Richard Procter wrote: Hi, This series of 29 small diffs slims pf.o by 2640 bytes and pf.c by 113 non-comment lines. We generally discuss one diff per mail. It makes it easier for people to comment and as you can imagine deal with possible conflicts in their tree :) As far as I understood, Richard provided step-by-step refactor diffs. What you want to discuss is a cumulative diff he referred to. No, what I want to discuss is one diff at a time not 29. That doesn't mean it has to be a cumulative diff.
Re: Don't be verbose in in6_update_ifa()
On Mon, Aug 17, 2015 at 01:06:32PM +0200, Martin Pieuchot wrote:
On 11/08/15(Tue) 16:25, Alexander Bluhm wrote:
On Mon, Aug 10, 2015 at 10:43:46PM +0200, Martin Pieuchot wrote:
In general these messages do not help. Here we have two cases of
messages, either logged at LOG_INFO or LOG_ERR.
Yes, multiple logs for the same error are bad. But there is a path
where we ignore the return code of in6_update_ifa().
in6_update_ifa()
in6_ifattach_loopback()
in6_ifattach()
The latter is called without error check from
if_up()
ifioctl()
ifnewlladdr()
in6_control()
So we should either propagate the error in these functions or add
a log somewhere in the call stack.
What about committing this diff first?
yes, OK bluhm@
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.359
diff -u -p -r1.359 if.c
--- net/if.c 16 Aug 2015 12:19:06 - 1.359
+++ net/if.c 17 Aug 2015 11:04:39 -
@@ -1314,11 +1314,11 @@ ifioctl(struct socket *so, u_long cmd, c
case AF_INET6:
s = splsoftnet();
if (cmd == SIOCIFAFATTACH)
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
else
in6_ifdetach(ifp);
splx(s);
- return (0);
+ return (error);
#endif /* INET6 */
default:
return (EAFNOSUPPORT);
@@ -1382,8 +1382,10 @@ ifioctl(struct socket *so, u_long cmd, c
#ifdef INET6
if (ISSET(ifr-ifr_flags, IFXF_AUTOCONF6)) {
s = splsoftnet();
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
splx(s);
+ if (error != 0)
+ return (error);
}
if ((ifr-ifr_flags IFXF_AUTOCONF6)
Index: netinet6//in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.162
diff -u -p -r1.162 in6.c
--- netinet6//in6.c 12 Aug 2015 09:06:18 - 1.162
+++ netinet6//in6.c 17 Aug 2015 11:00:04 -
@@ -479,7 +479,11 @@ in6_control(struct socket *so, u_long cm
* is no link-local yet.
*/
s = splsoftnet();
- in6_ifattach(ifp);
+ error = in6_ifattach(ifp);
+ if (error != 0) {
+ splx(s);
+ return (error);
+ }
error = in6_update_ifa(ifp, ifra, ia6);
splx(s);
if (error != 0)
Re: [Patch] pf refactoring
Martin Pieuchot mpi at openbsd.org writes: On 17/08/15(Mon) 17:39, Richard Procter wrote: Hi, This series of 29 small diffs slims pf.o by 2640 bytes and pf.c by 113 non-comment lines. We generally discuss one diff per mail. It makes it easier for people to comment and as you can imagine deal with possible conflicts in their tree :) As far as I understood, Richard provided step-by-step refactor diffs. What you want to discuss is a cumulative diff he referred to.
[patch] armv7/omap/ommmc.c: add ommmc_dump_regs
armv7 kernel will fail to compile with -DSDHC_DEBUG as it lacks a
definition for ommc_dump_regs, so add it.
I previously mailed this patch a while ago[1], but my mail client fudged
tabs/whitespace, here is a working patch.
Ian
[1] http://marc.info/?l=openbsd-techm=143580188423401w=2
Index: omap/ommmc.c
===
RCS file: /cvs/src/sys/arch/armv7/omap/ommmc.c,v
retrieving revision 1.14
diff -u -p -r1.14 ommmc.c
--- omap/ommmc.c30 May 2015 02:17:36 - 1.14
+++ omap/ommmc.c16 Aug 2015 22:01:31 -
@@ -244,7 +244,41 @@ void ommmc_write_data(struct ommmc_softc
#ifdef SDHC_DEBUG
int ommmcdebug = 20;
#define DPRINTF(n,s) do { if ((n) = ommmcdebug) printf s; } while (0)
-void ommmc_dump_regs(struct ommmc_softc *);
+void
+ommmc_dump_regs(struct ommmc_softc *)
+{
+ DPRINTF(3,(%s: SYSCONFIG: 0x%08x SYSSTATUS: 0x%08x CSRE: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_SYSCONFIG), HREAD4(sc,
MMCHS_SYSSTATUS),
+ HREAD4(sc, MMCHS_CSRE)));
+
+ DPRINTF(3,(%s: SYSTEST: 0x%08x CON: 0x%08x PWCNT: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_SYSTEST), HREAD4(sc,
MMCHS_CON),
+ HREAD4(sc, MMCHS_PWCNT)));
+
+ DPRINTF(3,(%s: BLK: 0x%08x ARG: 0x%08xCMD: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_BLK), HREAD4(sc, MMCHS_ARG),
+ HREAD4(sc, MMCHS_CMD)));
+
+ DPRINTF(3,(%s: RSP10: 0x%08x RSP32: 0x%08x RSP54: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_RSP10), HREAD4(sc,
MMCHS_RSP32),
+ HREAD4(sc, MMCHS_RSP54)));
+
+ DPRINTF(3,(%s: RSP76: 0x%08x DATA: 0x%08x PSTATE: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_RSP76), HREAD4(sc,
MMCHS_DATA),
+ HREAD4(sc, MMCHS_PSTATE)));
+
+ DPRINTF(3,(%s: HCTL: 0x%08xSYSCTL: 0x%08x STAT: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_HCTL), HREAD4(sc,
MMCHS_SYSCTL),
+ HREAD4(sc, MMCHS_STAT)));
+
+ DPRINTF(3,(%s:IE: 0x%08x ISE: 0x%08x AC12: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_IE), HREAD4(sc, MMCHS_ISE),
+ HREAD4(sc, MMCHS_AC12)));
+
+ DPRINTF(3,(%s: CAPA: 0x%08x CUR_CAPA: 0x%08xREV: 0x%08x\n,
+ DEVNAME(sc), HREAD4(sc, MMCHS_CAPA), HREAD4(sc,
MMCHS_CUR_CAPA),
+ HREAD4(sc, MMCHS_REV)));
+}
#else
#define DPRINTF(n,s) do {} while(0)
#endif
http(1) An alternate implementation for a subset of ftp(1)
Hi, http(1) is a drop-in substitute for ftp(1)'s AUTO-FETCHING FILES mode. It defaults to HTTP method when the url doesn't specify the protocol and supports transferring files from HTTP(S) and FTP servers. The FTP support is limited to file transfer and doesn't do command interpretation. It works[0] with pkg_add(1) and related tools when set as FETCH_CMD. A SMALL variant could be built by disabling FTP and HTTPS support. Source: http://www.nimmagadda.net/http.tar.gz Thanks Patrick Keshishian and sthen@ for spotting Host header bug in previous version. Comments/Suggestions to improve it are most welcome. [0] Needs this small pkg_add(1) diff to get rid of the error messages when fetching for a FTP server... http://marc.info/?l=openbsd-techm=143966672102753w=2
[PATCH] Fixes to sed(1) line buffering
The current sed(1) will not process a given line until it has read a
character of the following line (to detect EOF conditions). This means
that the following command
(echo foo; sleep 2; echo bar; sleep 2; echo baz; sleep 2; echo quux) | sed
Will wait two seconds before printing anything, and baz and quux will
print at the same time. The -u option does not change this behaviour.
While this isn't a mission-critical issue, it can be inconvenient when
using sed(1) to process interactive/slow network streams. None of the
other sed(1) implementations I can find exhibit this behaviour. I have
constructed a patch for the mf_fgets function whose logic is inspired by
NetBSDs implementation. I have not merely copied the NetBSD mf_fget, as
it has some funky logic regarding backup files that I do not
comprehend.
This is the first patch I have submitted to OpenBSD, so please tell me
if I have overlooked something.
Index: main.c
===
RCS file: /cvs/src/usr.bin/sed/main.c,v
retrieving revision 1.18
diff -u -p -r1.18 main.c
--- main.c 26 Nov 2014 18:34:51 - 1.18
+++ main.c 17 Aug 2015 12:57:58 -
@@ -262,30 +262,41 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
size_t len;
char *p;
int c;
+ static int firstfile = 1;
- if (f == NULL)
- /* Advance to first non-empty file */
- for (;;) {
- if (files == NULL) {
-lastline = 1;
-return (0);
- }
- if (files-fname == NULL) {
-f = stdin;
-fname = stdin;
- } else {
-fname = files-fname;
-if ((f = fopen(fname, r)) == NULL)
- err(FATAL, %s: %s,
- fname, strerror(errno));
- }
- if ((c = getc(f)) != EOF) {
-(void)ungetc(c, f);
-break;
- }
- (void)fclose(f);
+ /* Advance to next (possibly first) non-empty file */
+ for (;;) {
+ if (f != NULL (c = getc(f)) != EOF) {
+ (void)ungetc(c, f);
+ break;;
+ }
+ /* If we are here then either eof or no files are open yet */
+ if (f != NULL)
+ fclose(f);
+ if (firstfile == 0)
files = files-next;
+ else
+ firstfile = 0;
+ if (files == NULL) {
+ lastline = 1;
+ return (0);
+ }
+ if (files-fname == NULL) {
+ f = stdin;
+ fname = stdin;
+ } else {
+ fname = files-fname;
+ if ((f = fopen(fname, r)) == NULL)
+err(FATAL, %s: %s,
+fname, strerror(errno));
+ }
+ if ((c = getc(f)) != EOF) {
+ (void)ungetc(c, f);
+ break;
}
+ (void)fclose(f);
+ files = files-next;
+ }
if (lastline) {
sp-len = 0;
@@ -303,24 +314,6 @@ mf_fgets(SPACE *sp, enum e_spflag spflag
cspace(sp, p, len, spflag);
linenum++;
- /* Advance to next non-empty file */
- while ((c = getc(f)) == EOF) {
- (void)fclose(f);
- files = files-next;
- if (files == NULL) {
- lastline = 1;
- return (1);
- }
- if (files-fname == NULL) {
- f = stdin;
- fname = stdin;
- } else {
- fname = files-fname;
- if ((f = fopen(fname, r)) == NULL)
-err(FATAL, %s: %s, fname, strerror(errno));
- }
- }
- (void)ungetc(c, f);
return (1);
}
--
\ Troels
/\ Henriksen
Re: fix ld -Z on powerpc
I spent some time today figuting out why the binutils update broke ld
-Z on powerpc. Turns out to be a fairly thorny issue.
The new binutils discard empty setions. As a result the .gotpad0 and
.gotpad1 sections have disappeared. And a s a consequence the
__got_start and __got_end symbols are now absolute symbols as the
section they referenced to is no longer there. For example, an older
libc has:
845: 000eeb68 0 NOTYPE GLOBAL DEFAULT 17 __got_start
whereas -current has:
810: 000eeb58 0 NOTYPE GLOBAL DEFAULT ABS __got_start
On powerpc, crt0.o has weak references to __got_start and __got_end.
When building a binary with ld -Z, these are resolved to the absolute
symbols from libc. At runtime we then use these values, relocated as
if they were addresses within the binary itself, to change protections
and flush the instruction cache. This is very likely to result in a
segmentation fault.
There is probably a linker bug here, as it doesn't make any sense for
the linker to pick the address of these symbols from libc and stick it
into the binary. But I'm not sure about this. And it isn't all that
obvious what the fix would be. Is the bug that the symbols end up as
absolute? Or is the problem that it sibsequently resolves these to
the values from libc.so?
Wouldn't something like that address the problem better?
Index: elf.sc
===
RCS file: /OpenBSD/src/gnu/usr.bin/binutils-2.17/ld/scripttempl/elf.sc,v
retrieving revision 1.8
diff -u -p -r1.8 elf.sc
--- elf.sc 9 Aug 2014 04:49:47 - 1.8
+++ elf.sc 18 Aug 2015 05:02:41 -
@@ -195,10 +195,12 @@ if test $NO_PAD = y ; then
PAD_RO0=${RELOCATING+${RODATA_ALIGN} + ${RODATA_ALIGN_ADD_VAL};}
PAD_PLT0=${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} -
1));} .pltpad0 ${RELOCATING-0} : { ${RELOCATING+__plt_start = .;} }
PAD_PLT1=.pltpad1 ${RELOCATING-0} : { ${RELOCATING+__plt_end = .;}}
${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} - 1));}
- PAD_GOT0=${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} -
1));} .gotpad0 ${RELOCATING-0} : { ${RELOCATING+__got_start = .;} }
- PAD_GOT1=.gotpad1 ${RELOCATING-0} : { ${RELOCATING+__got_end = .;}}
${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} - 1));}
+ PAD_GOT0=${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} -
1));}
+ PAD_GOT1=${RELOCATING+. = ALIGN(${MAXPAGESIZE}) + (. (${MAXPAGESIZE} -
1));}
test $NO_PAD_CDTOR = y || PAD_CDTOR=
fi
+GOTSTART=PROVIDE (__got_start = .);
+GOTEND=PROVIDE (__got_end = .);
CTOR=.ctors${CONSTRUCTING-0} :
{
@@ -420,9 +422,11 @@ cat EOF
${OTHER_RELRO_SECTIONS}
${TEXT_DYNAMIC-${DYNAMIC}}
${DATA_GOT+${PAD_GOT+${PAD_GOT0}}}
+ ${DATA_GOT+${GOTSTART}}
${DATA_GOT+${DATA_NONEXEC_PLT+${PLT}}}
${DATA_GOT+${RELRO_NOW+${GOT}}}
${DATA_GOT+${RELRO_NOW+${GOTPLT}}}
+ ${DATA_GOT+${RELRO_NOW+${GOTEND}}}
${DATA_GOT+${RELRO_NOW+${PAD_GOT+${PAD_GOT1
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT+${GOT
/* If PAD_CDTOR, and separate .got and .got.plt sections, CTOR and DTOR
@@ -430,11 +434,13 @@ cat EOF
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT+${PAD_CDTOR+${RELOCATING+${CTOR}}
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT+${PAD_CDTOR+${RELOCATING+${DTOR}}
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT+${PAD_GOT+${PAD_GOT1}
+ ${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT+${GOTEND
${RELOCATING+${DATA_SEGMENT_RELRO_END}}
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT-${GOT
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT-${PAD_CDTOR+${RELOCATING+${CTOR}}
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT-${PAD_CDTOR+${RELOCATING+${DTOR}}
${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT-${PAD_GOT+${PAD_GOT1}
+ ${DATA_GOT+${RELRO_NOW-${SEPARATE_GOTPLT-${GOTEND
${DATA_GOT+${RELRO_NOW-${GOTPLT}}}
${DATA_NONEXEC_PLT-${DATA_PLT+${PLT_BEFORE_GOT-${PAD_PLT+${PAD_PLT0}
@@ -458,6 +464,7 @@ cat EOF
${DATA_NONEXEC_PLT-${DATA_PLT+${PLT_BEFORE_GOT+${PLT
${DATA_NONEXEC_PLT-${DATA_PLT+${PLT_BEFORE_GOT+${PAD_PLT+${PAD_PLT1}
${SDATA_GOT+${PAD_GOT+${PAD_GOT0}}}
+ ${SDATA_GOT+${GOTSTART}}
${SDATA_GOT+${DATA_NONEXEC_PLT+${PLT}}}
${SDATA_GOT+${RELOCATING+${OTHER_GOT_SYMBOLS}}}
${SDATA_GOT+${GOT}}
@@ -468,6 +475,7 @@ cat EOF
${DATA_GOT-${PAD_CDTOR+${RELOCATING+${DTOR
${SDATA_GOT+${OTHER_GOT_SECTIONS}}
+ ${SDATA_GOT+${GOTEND}}
${SDATA_GOT+${PAD_GOT+${PAD_GOT1}}}
${SDATA}
Re: rtfree(9) and nd6_prefix_{on,off}link
On Mon, Aug 17, 2015 at 12:34:13PM +0200, Martin Pieuchot wrote:
Ultimately my goal is to use rt_ifa_{add,del}() instead of
nd6_prefix_{on,off}link() but right now I need to remove the
rt-ref_cnt--.
@@ -1861,26 +1856,11 @@ nd6_prefix_onlink(struct nd_prefix *pr)
info.rti_info[RTAX_NETMASK] = sin6tosa(mask6);
error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, rt, ifp-if_rdomain);
- if (error == 0) {
- if (rt != NULL) /* this should be non NULL, though */
- rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
+ if (error == 0 rt != NULL) {
pr-ndpr_stateflags |= NDPRF_ONLINK;
Here you change the check for setting ndpr_stateflags from (error
== 0) to (error == 0 rt != NULL). Although I think that both
checks have the same result, would it be better to use the same
logic as in defrouter_addreq()?
if (rt) {
rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
rtfree(rt);
}
if (error == 0)
pr-ndpr_stateflags |= NDPRF_ONLINK;
bluhm
Re: rtfree(9) and nd6_prefix_{on,off}link
On 17/08/15(Mon) 18:25, Alexander Bluhm wrote:
On Mon, Aug 17, 2015 at 12:34:13PM +0200, Martin Pieuchot wrote:
Ultimately my goal is to use rt_ifa_{add,del}() instead of
nd6_prefix_{on,off}link() but right now I need to remove the
rt-ref_cnt--.
@@ -1861,26 +1856,11 @@ nd6_prefix_onlink(struct nd_prefix *pr)
info.rti_info[RTAX_NETMASK] = sin6tosa(mask6);
error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, rt, ifp-if_rdomain);
- if (error == 0) {
- if (rt != NULL) /* this should be non NULL, though */
- rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
+ if (error == 0 rt != NULL) {
pr-ndpr_stateflags |= NDPRF_ONLINK;
Here you change the check for setting ndpr_stateflags from (error
== 0) to (error == 0 rt != NULL). Although I think that both
checks have the same result, would it be better to use the same
logic as in defrouter_addreq()?
I'm changing the logic to match what's done in rt_ifa_add() but if you
look at rtrequest1(RTM_ADD, ...) you'll see that rt cannot be NULL when
error is 0.
I think it is be better to have the same check everywhere and I don't
care if it is (error != 0), (rt == NULL) or both.
Re: rtfree(9) and nd6_prefix_{on,off}link
On Mon, Aug 17, 2015 at 07:03:55PM +0200, Martin Pieuchot wrote:
On 17/08/15(Mon) 18:25, Alexander Bluhm wrote:
On Mon, Aug 17, 2015 at 12:34:13PM +0200, Martin Pieuchot wrote:
Ultimately my goal is to use rt_ifa_{add,del}() instead of
nd6_prefix_{on,off}link() but right now I need to remove the
rt-ref_cnt--.
@@ -1861,26 +1856,11 @@ nd6_prefix_onlink(struct nd_prefix *pr)
info.rti_info[RTAX_NETMASK] = sin6tosa(mask6);
error = rtrequest1(RTM_ADD, info, RTP_CONNECTED, rt, ifp-if_rdomain);
- if (error == 0) {
- if (rt != NULL) /* this should be non NULL, though */
- rt_sendmsg(rt, RTM_ADD, ifp-if_rdomain);
+ if (error == 0 rt != NULL) {
pr-ndpr_stateflags |= NDPRF_ONLINK;
Here you change the check for setting ndpr_stateflags from (error
== 0) to (error == 0 rt != NULL). Although I think that both
checks have the same result, would it be better to use the same
logic as in defrouter_addreq()?
I'm changing the logic to match what's done in rt_ifa_add() but if you
look at rtrequest1(RTM_ADD, ...) you'll see that rt cannot be NULL when
error is 0.
I think it is be better to have the same check everywhere and I don't
care if it is (error != 0), (rt == NULL) or both.
I also want to have the same logic everywhere, it is quite inconsistent
now.
I think the best would be to check only for (error == 0). Then it
is clear that rtrequest1() guarantees rt != NULL. The man page
rtrequest1(9) should mention this.
If we agree upon this, I can make a diff to unify the caller.
bluhm
Re: http(1) An alternate implementation for a subset of ftp(1)
On Mon, Aug 17, 2015 at 06:06:17PM +0200, Sebastien Marie wrote:
Hi,
I start reading your code, and I have a first remark.
I see in main.c (at line 142 and next) that on redirection, you trust
the server for the filename. I am not sure it is a good thing to do.
If the user request 'http://www.example.com/a_filename' (without -o),
the file created should be 'a_filename' what ever the redirection is.
Else, a evil server could arbitrary choose the filename (in the current
directory), and as file creation is done with O_TRUNC (or O_APPEND in
resume case), an evil server could override the file he wants.
Thanks for the comments, I agree with your observation. This diff
evaluates filename just once.
Index: main.c
===
RCS file: /cvs/http/main.c,v
retrieving revision 1.67
diff -u -p -r1.67 main.c
--- main.c 16 Aug 2015 08:00:25 - 1.67
+++ main.c 17 Aug 2015 17:33:20 -
@@ -108,8 +108,8 @@ main(int argc, char *argv[])
}
for (i = 0; i argc; i++) {
-retry:
fn = (output) ? output : basename(argv[i]);
+retry:
url_str = url_encode(argv[i]);
p = url_type(url_str);
if (url_parse(url_str, url, p) != 0)
Partially unlock the em(4) interrupt handler
The diff below is a first step towards running the em(4) interrupt
handler without grabbing the kernel lock. It runs the rx completion
path without the lock, which is the important bit to be able to test
the work that has been going on to make the network stack run on
multiple CPUs. But the handler still grabs the kernel lock for tx
completions, rx ring refills and everything else. Which means that it
will still grab the lock every time it runs, which may affect how it
interacts with the network stack.
The locking strategy is centered around the if_rxring interface. The
tricky bit is to prevent the code that brings down the interface from
frees the data structures accessed by the interrupt handler while it
is running. The interrupt handler's rx completion path checks if
there are any filled ring slots before checking them and holds the
lock while doing so. The code that frees the rx ring now explicitly
calls
if_rxr_init(sc-rx_ring, 0, 0);
which clears the number of filled ring slots, so after the
initialization call completes, we can be sure the rx completion code
doesn't run again until we refill the ring.
The nice side effect if resetting the if_rxring structure this way is
that if you bring down the interface, systat mbuf correctly shows the
number of allocated mbufs on the ring as 0. But it also resets the
water marks, which may or may not be a good idea.
ok?
Index: if_em.c
===
RCS file: /cvs/src/sys/dev/pci/if_em.c,v
retrieving revision 1.299
diff -u -p -r1.299 if_em.c
--- if_em.c 24 Jun 2015 09:40:54 - 1.299
+++ if_em.c 17 Aug 2015 09:24:58 -
@@ -353,6 +353,8 @@ em_attach(struct device *parent, struct
sc = (struct em_softc *)self;
sc-osdep.em_pa = *pa;
+ mtx_init(sc-rx_mtx, IPL_NET);
+
timeout_set(sc-timer_handle, em_local_timer, sc);
timeout_set(sc-tx_fifo_timer_handle, em_82547_move_tail, sc);
@@ -922,6 +924,8 @@ em_intr(void *arg)
refill = 1;
}
+ KERNEL_LOCK();
+
/* Link status change */
if (reg_icr (E1000_ICR_RXSEQ | E1000_ICR_LSC)) {
sc-hw.get_link_status = 1;
@@ -942,6 +946,8 @@ em_intr(void *arg)
E1000_WRITE_REG(sc-hw, RDT, sc-last_rx_desc_filled);
}
+ KERNEL_UNLOCK();
+
return (1);
}
@@ -1698,8 +1704,8 @@ em_allocate_pci_resources(struct em_soft
sc-hw.back = sc-osdep;
intrstr = pci_intr_string(pc, ih);
- sc-sc_intrhand = pci_intr_establish(pc, ih, IPL_NET, em_intr, sc,
- sc-sc_dv.dv_xname);
+ sc-sc_intrhand = pci_intr_establish(pc, ih, IPL_NET | IPL_MPSAFE,
+ em_intr, sc, sc-sc_dv.dv_xname);
if (sc-sc_intrhand == NULL) {
printf(: couldn't establish interrupt);
if (intrstr != NULL)
@@ -2413,6 +2419,8 @@ em_txeof(struct em_softc *sc)
if (sc-num_tx_desc_avail == sc-num_tx_desc)
return;
+ KERNEL_LOCK();
+
num_avail = sc-num_tx_desc_avail;
first = sc-next_tx_to_clean;
tx_desc = sc-tx_desc_base[first];
@@ -2494,6 +2502,8 @@ em_txeof(struct em_softc *sc)
ifp-if_timer = EM_TX_TIMEOUT;
sc-num_tx_desc_avail = num_avail;
+
+ KERNEL_UNLOCK();
}
/*
@@ -2743,6 +2753,10 @@ em_free_receive_structures(struct em_sof
INIT_DEBUGOUT(free_receive_structures: begin);
+ mtx_enter(sc-rx_mtx);
+ if_rxr_init(sc-rx_ring, 0, 0);
+ mtx_leave(sc-rx_mtx);
+
if (sc-rx_buffer_area != NULL) {
rx_buffer = sc-rx_buffer_area;
for (i = 0; i sc-num_rx_desc; i++, rx_buffer++) {
@@ -2825,6 +2839,8 @@ em_rxfill(struct em_softc *sc)
int post = 0;
int i;
+ mtx_enter(sc-rx_mtx);
+
i = sc-last_rx_desc_filled;
for (slots = if_rxr_get(sc-rx_ring, sc-num_rx_desc);
@@ -2841,6 +2857,8 @@ em_rxfill(struct em_softc *sc)
if_rxr_put(sc-rx_ring, slots);
+ mtx_leave(sc-rx_mtx);
+
return (post);
}
@@ -2856,6 +2874,7 @@ em_rxeof(struct em_softc *sc)
{
struct ifnet*ifp = sc-interface_data.ac_if;
struct mbuf_listml = MBUF_LIST_INITIALIZER();
+ struct mbuf_listfree_ml = MBUF_LIST_INITIALIZER();
struct mbuf *m;
u_int8_taccept_frame = 0;
u_int8_teop = 0;
@@ -2867,8 +2886,11 @@ em_rxeof(struct em_softc *sc)
struct em_buffer*pkt;
u_int8_tstatus;
- if (if_rxr_inuse(sc-rx_ring) == 0)
+ mtx_enter(sc-rx_mtx);
+ if (if_rxr_inuse(sc-rx_ring) == 0) {
+ mtx_leave(sc-rx_mtx);
return;
+ }
i = sc-next_rx_desc_to_check;
@@ -2988,12 +3010,12 @@ em_rxeof(struct em_softc *sc)
Re: http(1) An alternate implementation for a subset of ftp(1)
Hi,
I attached 3 patchs:
- retry.patch: move name of output file outside the retry loop. Should
keep the filename defined by user, instead of pike one from server.
- spell.patch: correct a small spelling error
- url-user.patch: a small change in url_parse() in order to copte with
@ char in url path (http://example.com/my@funny@path isn't well
parsed)
And another note, the url_parse() don't copte with url in ipv6
(for example http://[2a00:1450:4007:807::1010]:80/), but I don't known
if it is a real issue for now.
Thanks.
--
Sebastien Marie
Index: b/main.c
===
--- a/main.c2015-08-16 10:00:25.0 +0200
+++ b/main.c2015-08-17 18:48:08.951532825 +0200
@@ -108,8 +108,8 @@ main(int argc, char *argv[])
}
for (i = 0; i argc; i++) {
-retry:
fn = (output) ? output : basename(argv[i]);
+retry:
url_str = url_encode(argv[i]);
p = url_type(url_str);
if (url_parse(url_str, url, p) != 0)
Index: b/main.c
===
--- a/main.c2015-08-17 18:48:08.951532825 +0200
+++ b/main.c2015-08-17 19:32:54.811047138 +0200
@@ -119,7 +119,7 @@ retry:
(void)strlcpy(url.port, port, sizeof(url.port));
if (strcmp(url.path, /) == 0 || strlen(url.path) == 0)
- errx(1, No filename afer host: %s, url.host);
+ errx(1, No filename after host: %s, url.host);
if (url_connect(url, pproxy) == -1)
return (-1);
Index: b/util.c
===
--- a/util.c2015-08-17 19:29:39.239294584 +0200
+++ b/util.c2015-08-17 19:37:40.483642472 +0200
@@ -198,6 +198,16 @@ url_parse(const char *url_str, struct ur
else
s = curl;
+ /* extract url path */
+ if ((e = strchr(s, '/'))) {
+ if (strlcpy(url-path, e,
+ sizeof(url-path)) = sizeof(url-path)) {
+ warnx(url_parse: path overflow);
+ goto cleanup;
+ }
+ *e = '\0';
+ }
+
/* extract user and password */
if ((e = strchr(s, '@'))) {
*e++ = '\0';
@@ -221,16 +231,6 @@ url_parse(const char *url_str, struct ur
s = e;
}
- /* extract url path */
- if ((e = strchr(s, '/'))) {
- if (strlcpy(url-path, e,
- sizeof(url-path)) = sizeof(url-path)) {
- warnx(url_parse: path overflow);
- goto cleanup;
- }
- *e = '\0';
- }
-
/* extract url port */
if ((t = strchr(s, ':'))) {
*t++ = '\0';
Re: Return a rtentry* in arplookup()
On Mon, Aug 17, 2015 at 12:31:20PM +0200, Martin Pieuchot wrote:
This brings arplookup() in sync with nd6_lookup() and is necessary for
upcoming struct rtentry reference count.
@@ -633,9 +633,10 @@ in_arpinput(struct mbuf *m)
itaddr = myaddr;
goto reply;
}
- la = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0,
+ rt = arplookup(isaddr.s_addr, itaddr.s_addr == myaddr.s_addr, 0,
rtable_l2(m-m_pkthdr.ph_rtableid));
- if (la (rt = la-la_rt) (sdl = SDL(rt-rt_gateway))) {
+ if (rt != NULL (la = (struct llinfo_arp *)rt-rt_llinfo)
+ (sdl = SDL(rt-rt_gateway))) {
I think the la != NULL check in this if condition is not neccessary
as (rt-rt_flags RTF_LLINFO) has been checked in arplookup().
@@ -818,7 +818,7 @@ arplookup(u_int32_t addr, int create, in
}
return (0);
}
- return ((struct llinfo_arp *)rt-rt_llinfo);
+ return (rt);
}
Could you convert the two return (0) in arplookup() into return (NULL)?
Anyway OK bluhm@
Fwd: worm.c removing unused variables
Hello all,
I would like to suggest these changes to remove unused variables
and a respectively unnecessary call of the gettimeofday, also removes
a casting in the malloc, that seems to be unnecessary.
Index: worm.c
===
RCS file: /cvs/src/games/worm/worm.c,v
retrieving revision 1.28
diff -u -p -r1.28 worm.c
--- worm.c 9 Mar 2015 19:52:02 - 1.28
+++ worm.c 11 Aug 2015 18:39:55 -
@@ -88,8 +88,7 @@ int
main(int argc, char **argv)
{
int retval;
- struct timeval t, tod;
- struct timezone tz;
+ struct timeval t;
struct pollfd pfd[1];
const char *errstr;
@@ -162,7 +161,6 @@ main(int argc, char **argv)
/* Delay could be a command line option */
t.tv_sec = 1;
t.tv_usec = 0;
- (void)gettimeofday(tod, tz);
pfd[0].fd = STDIN_FILENO;
pfd[0].events = POLLIN;
retval = poll(pfd, 1, t.tv_sec * 1000 + t.tv_usec / 1000);
@@ -335,7 +333,7 @@ newlink(void)
{
struct body *tmp;
- if ((tmp = (struct body *) malloc(sizeof (struct body))) == NULL) {
+ if ((tmp = malloc(sizeof (struct body))) == NULL) {
endwin();
errx(1, out of memory);
}
*att.*
* Vinicios Barros,*
*Undergrad Computer Engineering at PUCRS*
*IRC freenode/OFTC vbarros*
*twitter: @viniciosbarros*
**
Re: rtfree(9) conversion in rt_getifa()
On Mon, Aug 17, 2015 at 12:29:46PM +0200, Martin Pieuchot wrote:
Here a route lookup is done to find an existing ``ifa'' in order to
attach a new route.
This code runs under KERNEL_LOCK and returns a route entry found in
the table (RT_VALID), so rtfree(9) will not free the route.
Note that it is safe to rtfree() a route entry while keeping a pointer
to a *valid* ``ifa'' because ifa insertion/removal are serialized by
the KERNEL_LOCK. Yes, route entries increment ifas refcounter but this
only matters for stall ifas, and there's already a check for that here.
Ok?
OK bluhm@
Index: net/route.c
===
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.220
diff -u -p -r1.220 route.c
--- net/route.c 17 Aug 2015 09:50:12 - 1.220
+++ net/route.c 17 Aug 2015 10:25:24 -
@@ -666,14 +666,18 @@ ifa_ifwithroute(int flags, struct sockad
struct rtentry *rt = rtalloc(gateway, 0, rtableid);
if (rt == NULL)
return (NULL);
- rt-rt_refcnt--;
/* The gateway must be local if the same address family. */
if ((rt-rt_flags RTF_GATEWAY)
- rt_key(rt)-sa_family == dst-sa_family)
+ rt_key(rt)-sa_family == dst-sa_family) {
+ rtfree(rt);
return (NULL);
+ }
ifa = rt-rt_ifa;
- if (ifa == NULL || ifa-ifa_ifp == NULL)
+ if (ifa == NULL || ifa-ifa_ifp == NULL) {
+ rtfree(rt);
return (NULL);
+ }
+ rtfree(rt);
}
if (ifa-ifa_addr-sa_family != dst-sa_family) {
struct ifaddr *oifa = ifa;
Re: [PATCH] Fixes to sed(1) line buffering
Actually, ignore this, it seems to have been fixed (by accident?) by the recent implementation of the -i flag in CURRENT. -- \ Troels /\ Henriksen
netcat non blocking socket
Hi,
When running my pf regression tests, I triggered netcat hanging in
write(2). This is bit strange as poll(2) should check that the socket
is writeable. However making the network socket non-blocking
keeps the data flowing. Note that the code to handle EAGAIN is
already there.
To trigger the blocking write(2) I use:
openssl rand 20 | nc -N tcp-echo-server 7 | wc -c | grep 20$
Ktrace shows that it happens from time to time:
11789 nc CALL poll(0x7f7bd5e0,4,INFTIM)
11789 nc RET poll 2
11789 nc CALL write(3,0x7f7bd610,0x4000)
11789 nc RET write -1 errno 35 Resource temporarily unavailable
11789 nc CALL read(3,0x7f7c1610,0x4000)
11789 nc GIO fd 3 read 2048 bytes
ok?
bluhm
Index: usr.bin/nc/netcat.c
===
RCS file: /data/mirror/openbsd/cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.130
diff -u -p -r1.130 netcat.c
--- usr.bin/nc/netcat.c 26 Jul 2015 19:12:28 - 1.130
+++ usr.bin/nc/netcat.c 17 Aug 2015 07:25:30 -
@@ -95,7 +95,7 @@ int Sflag; /* TCP MD5
signature opti
intTflag = -1; /* IP Type of Service */
intrtableid = -1;
-int timeout = -1;
+int timeout = INFTIM;
int family = AF_UNSPEC;
char *portlist[PORT_MAX+1];
char *unix_dg_tmp_socket;
@@ -397,8 +397,8 @@ main(int argc, char *argv[])
readwrite(s);
} else {
len = sizeof(cliaddr);
- connfd = accept(s, (struct sockaddr *)cliaddr,
- len);
+ connfd = accept4(s, (struct sockaddr *)cliaddr,
+ len, SOCK_NONBLOCK);
if (connfd == -1) {
/* For now, all errnos are fatal */
err(1, accept);
@@ -594,8 +594,8 @@ remote_connect(const char *host, const c
res0 = res;
do {
- if ((s = socket(res0-ai_family, res0-ai_socktype,
- res0-ai_protocol)) 0)
+ if ((s = socket(res0-ai_family, res0-ai_socktype |
+ SOCK_NONBLOCK, res0-ai_protocol)) 0)
continue;
if (rtableid = 0 (setsockopt(s, SOL_SOCKET, SO_RTABLE,
@@ -644,15 +644,9 @@ timeout_connect(int s, const struct sock
{
struct pollfd pfd;
socklen_t optlen;
- int flags, optval;
+ int optval;
int ret;
- if (timeout != -1) {
- flags = fcntl(s, F_GETFL, 0);
- if (fcntl(s, F_SETFL, flags | O_NONBLOCK) == -1)
- err(1, set non-blocking mode);
- }
-
if ((ret = connect(s, name, namelen)) != 0 errno == EINPROGRESS) {
pfd.fd = s;
pfd.events = POLLOUT;
@@ -669,9 +663,6 @@ timeout_connect(int s, const struct sock
} else
err(1, poll failed);
}
-
- if (timeout != -1 fcntl(s, F_SETFL, flags) == -1)
- err(1, restoring flags);
return (ret);
}
patch fixes IPv6 reassembly when packet hits PBR rule
Hello,
I'm sending finalized patch. The final shape has been discussed with bluhm@,
who was kind enough to do thorough testing on OpenBSD.
The patch solves the problem for deployment as follows:
Client -- MTU_1 -- PF -- MTU_1 -- Router -- MTU_2 -- Server
MTU_2 = MTU_1/2
PF does IPv6 reassembly for PBRed packets (packets forwarded on behalf of
route-to action). Let's assume client sends packet of MTU_1 size. PF forwards
packet to destination. Router discards packet, because it does not
fit to wire. It sends packet too big suggesting MTU_2.
Client retries with fragmented packet this time. In our scenario it uses
MTU_1/2. PF reassembles the packet and is going to forwarded to destination
as ordered by route-to action. Snippet below comes from pf_route6():
5710/*
5711 * If the packet is too large for the outgoing interface,
5712 * send back an icmp6 error.
5713 */
5714if (IN6_IS_SCOPE_EMBED(dst-sin6_addr))
5715dst-sin6_addr.s6_addr16[1] = htons(ifp-if_index);
5716if ((u_long)m0-m_pkthdr.len = ifp-if_mtu) {
5717nd6_output(ifp, m0, dst, NULL);
5718} else {
5719in6_ifstat_inc(ifp, ifs6_in_toobig);
5720if (r-rt != PF_DUPTO)
5721icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0,
ifp-if_mtu);
5722else
5723goto bad;
5724}
test at line 5716 is invalid for our scenario. In our case condition
is met, so PF puts IPv6 reassembled packet to wire, while it should turn
the packet back to fragments as sent by client.
The fix is straightforward. Whenever pf_route6() deals with reassembled
packet it must use pf_refragment6() to turn it back to fragments.
pf_refragment6() must use nd6_output() to put fragment to wire, if it is
invoked on behalf of pf_route6().
bluhm@ exchanged couple of emails with me to finalize the patch I intend to
commit. These are the two points I'd like to highlight:
- PF should report ICMP packet too big for every fragment
if it fails to put to wire (1)
- the dup-to actions should not be treated as special case
curent version does not send ICMP pakcet too big for duped
packets. proposed patch changes that (2)
Ad (1) the earlier patch version I've sent in June sends only one ICMP packet
too big message as it hits the first for the first fragment, which exceeds MTU,
all other fragments are discarded only. This would be a deviation to regular
IPv6 router, which sends MTU exceeded to every packet, which exceeds MTU. The
patch proposed here makes PF to act as regular router.
Ad (2) dup-to actions: we assume the dup-to is used to pass duped packets to
IDS deployed somewhere on network. So if duped packet exceeds MTU we let PF to
send ICMP packet too big back to packet source, so it will somewhat enforce
correct fragment size to keep IDS in picture. Still the dup-to usecase is kind
of blurry to me, if someone can point me to yet another use case for dup-to
I'll be glad.
I believe the what we have is optimal fix.
regards
sasha
8---8---8--8
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.935
diff -u -p -r1.935 pf.c
--- pf.c21 Jul 2015 02:32:04 - 1.935
+++ pf.c17 Aug 2015 17:58:55 -
@@ -5633,6 +5633,7 @@ pf_route6(struct mbuf **m, struct pf_rul
struct ifnet*ifp = NULL;
struct pf_addr naddr;
struct pf_src_node *sns[PF_SN_MAX];
+ struct m_tag*mtag;
if (m == NULL || *m == NULL || r == NULL ||
(dir != PF_IN dir != PF_OUT) || oifp == NULL)
@@ -5707,20 +5708,20 @@ pf_route6(struct mbuf **m, struct pf_rul
in6_proto_cksum_out(m0, ifp);
- /*
-* If the packet is too large for the outgoing interface,
-* send back an icmp6 error.
-*/
if (IN6_IS_SCOPE_EMBED(dst-sin6_addr))
dst-sin6_addr.s6_addr16[1] = htons(ifp-if_index);
- if ((u_long)m0-m_pkthdr.len = ifp-if_mtu) {
+
+ /*
+* If packet has been reassembled by PF earlier, we have to
+* use pf_refragment6() here to turn it back to fragments.
+*/
+ if ((mtag = m_tag_find(m0, PACKET_TAG_PF_REASSEMBLED, NULL))) {
+ (void) pf_refragment6(m0, mtag, dst, ifp);
+ } else if ((u_long)m0-m_pkthdr.len = ifp-if_mtu) {
nd6_output(ifp, m0, dst, NULL);
} else {
in6_ifstat_inc(ifp, ifs6_in_toobig);
- if (r-rt != PF_DUPTO)
- icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0, ifp-if_mtu);
- else
- goto bad;
+ icmp6_error(m0, ICMP6_PACKET_TOO_BIG, 0, ifp-if_mtu);
}
done:
Re: fix ld -Z on powerpc
Date: Wed, 12 Aug 2015 15:48:57 +0200 (CEST) From: Mark Kettenis mark.kette...@xs4all.nl I spent some time today figuting out why the binutils update broke ld -Z on powerpc. Turns out to be a fairly thorny issue. The new binutils discard empty setions. As a result the .gotpad0 and .gotpad1 sections have disappeared. And a s a consequence the __got_start and __got_end symbols are now absolute symbols as the section they referenced to is no longer there. For example, an older libc has: 845: 000eeb68 0 NOTYPE GLOBAL DEFAULT 17 __got_start whereas -current has: 810: 000eeb58 0 NOTYPE GLOBAL DEFAULT ABS __got_start On powerpc, crt0.o has weak references to __got_start and __got_end. When building a binary with ld -Z, these are resolved to the absolute symbols from libc. At runtime we then use these values, relocated as if they were addresses within the binary itself, to change protections and flush the instruction cache. This is very likely to result in a segmentation fault. There is probably a linker bug here, as it doesn't make any sense for the linker to pick the address of these symbols from libc and stick it into the binary. But I'm not sure about this. And it isn't all that obvious what the fix would be. Is the bug that the symbols end up as absolute? Or is the problem that it sibsequently resolves these to the values from libc.so? It does point out a fundamental weakness about the approach we've taken with the __plt_start/end and __got_start/end synbols. They work fine of you use something like dlsym(3) to lookup the value for a specific object, but if you rely on the default symbol resolution, it isn't clear if you get the right version. Therefore I think that the powerpc crt0.o code shouldn't be doing what it is doing. The diff below removes that code. This diff has a downside though. The GOT on -static -nopie binaries will no longer be read-only. I don't think that is a big loss, as -static -pie binaries are the default now and those do get a read-only GOT. If we think a read-only GOT for -static binaries is still important, there are a few other potential options to achive this that don't need to rely on __got_start/end. ping? This will also make my life easier with the secure-plt changes that are in the pipeline. Index: powerpc/md_init.h === RCS file: /cvs/src/lib/csu/powerpc/md_init.h,v retrieving revision 1.3 diff -u -p -r1.3 md_init.h --- powerpc/md_init.h 26 Dec 2014 13:52:01 - 1.3 +++ powerpc/md_init.h 12 Aug 2015 13:08:28 - @@ -64,88 +64,20 @@ #include sys/syscall.h /* for SYS_mprotect */ #define STR(x) __STRING(x) + #define MD_CRT0_START \ __asm( \ .text \n \ .section\.text\ \n \ .align 2\n \ -.size __got_start, 0 \n \ -.type __got_start, @object\n \ -.size __got_end, 0\n \ -.type __got_end, @object \n \ -.weak __got_start \n \ -.weak __got_end \n \ .globl _start \n \ .type _start, @function \n \ .globl __start \n \ .type __start, @function \n \ _start: \n \ __start:\n \ -# move argument registers to saved registers for startup flush \n \ -# ...except r6 (auxv) as ___start() doesn't need it \n \ -mr %r25, %r3\n \ -mr %r24, %r4\n \ -mr %r23, %r5\n \ -mr %r22, %r7\n \ -mflr%r27/* save off old link register */\n \ -bl 1f \n \ -# this instruction never gets executed but can be used \n \ -# to find the virtual address where the page is loaded. \n \ -bl _GLOBAL_OFFSET_TABLE_@local-4\n \ -1: \n \ -mflr%r6 # this
Re: http(1) An alternate implementation for a subset of ftp(1)
Hi, I start reading your code, and I have a first remark. I see in main.c (at line 142 and next) that on redirection, you trust the server for the filename. I am not sure it is a good thing to do. If the user request 'http://www.example.com/a_filename' (without -o), the file created should be 'a_filename' what ever the redirection is. Else, a evil server could arbitrary choose the filename (in the current directory), and as file creation is done with O_TRUNC (or O_APPEND in resume case), an evil server could override the file he wants. Regards. -- Sebastien Marie
