Re: ld.so: NULL dereference on corrupt library
On Tue, May 4, 2021 at 12:43 PM Klemens Nanni wrote: ... > I compared my corrupted shared library with an intact copy from ports > and it showed that the corrupted one was simply zeroed out at some point > (not truncated) until the end, e.g. readelf(1)'s `-h' or `-l' report > "Error: no .dynamic section in the dynamic segment". > > So this isn't a case of some badly linked library or one that has a few > bits flipped, it's simply a partial one... seems like bad luck? > IMHO, the benefit of adding this check is almost zero: it gives a slightly better experience for a small set of possible data corruption cases, when similar corruptions that affect other pages aren't helped at all as it'll crash when it executes zeroed text, or accesses zeroed data, or fails to find a required symbol because the symbol table was zeroed out. If we want to protect against that sort of hardware lossage, then a filesystem which does so is the way to go, not an alarm on one window of a glass house. Philip Guenther
Re: make rsync -v less verbose
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 20:03:19 +0200:
> I like rsync -v but hell it is noisy with openrsync.
> Just shut up about all the files that have not changed unless you go -vv.
Before we do this, are there reasons to keep this like it is in the original?
I think i actually made it do this.
If you really think it helps, ok benno@
>
> --
> :wq Claudio
>
> Index: downloader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/downloader.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 downloader.c
> --- downloader.c 8 May 2019 21:30:11 - 1.21
> +++ downloader.c 5 May 2021 17:56:20 -
> @@ -85,7 +85,7 @@ log_file(struct sess *sess,
> if (sess->opts->server)
> return;
>
> - frac = (dl->total == 0) ? 100.0 :
> + frac = (dl->total == 0) ? 0.0 :
> 100.0 * dl->downloaded / dl->total;
>
> if (dl->total > 1024 * 1024 * 1024) {
> @@ -102,8 +102,12 @@ log_file(struct sess *sess,
> unit = "KB";
> }
>
> - LOG1("%s (%.*f %s, %.1f%% downloaded)",
> - f->path, prec, tot, unit, frac);
> + if (dl->downloaded > 0)
> + LOG1("%s (%.*f %s, %.1f%% downloaded)",
> + f->path, prec, tot, unit, frac);
> + else
> + LOG2("%s (%.*f %s, %.1f%% downloaded)",
> + f->path, prec, tot, unit, frac);
> }
>
> /*
>
Re: rpki-client: change "asn" from string to integer in JSON output
Job Snijders(j...@openbsd.org) on 2021.05.05 16:35:46 +:
> I'd like to modify our JSON format, many people in the community have
> voiced complaints that transforming the string to an integer is
> annoying.
>
> This won't break existing deployments coupled with GoRTR.
>
> OK?
ok benno@
>
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 output-json.c
> --- output-json.c 8 Apr 2021 19:49:27 - 1.15
> +++ output-json.c 5 May 2021 15:29:15 -
> @@ -100,7 +100,7 @@ output_json(FILE *out, struct vrp_tree *
>
> ip_addr_print(>addr, v->afi, buf, sizeof(buf));
>
> - if (fprintf(out, "\t\t{ \"asn\": \"AS%u\", \"prefix\": \"%s\", "
> + if (fprintf(out, "\t\t{ \"asn\": %u, \"prefix\": \"%s\", "
> "\"maxLength\": %u, \"ta\": \"%s\" }",
> v->asid, buf, v->maxlength, v->tal) < 0)
> return -1;
>
Re: simplify the openrsync uploader
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 17:53:20 +0200:
> The rsync uploader (what is the generator in rsync) can be simplified and
> cleaned up a fair bit.
>
> There is some confusion of non-blocking IO on regular files and the idea
> to poll() between openat() and fstat(). This is all not needed and
> therefor a lot of the code handling files can be moved into pre_file.
> This also removes the UPLOAD_READ_LOCAL state since it is no longer
> needed.
>
> As a little extra gift this diff also plugs a mem leak in rsync_uploader.
> The mbuf buffer was not freed and so for every file being checksummed it
> would leak one of those.
see below about that.
Other than that its ok.
>
> --
> :wq Claudio
>
> Index: uploader.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 uploader.c
> --- uploader.c22 Mar 2021 11:20:04 - 1.24
> +++ uploader.c5 May 2021 15:39:10 -
> @@ -33,8 +33,7 @@
>
> enum uploadst {
> UPLOAD_FIND_NEXT = 0, /* find next to upload to sender */
> - UPLOAD_WRITE_LOCAL, /* wait to write to sender */
> - UPLOAD_READ_LOCAL, /* wait to read from local file */
> + UPLOAD_WRITE, /* wait to write to sender */
> UPLOAD_FINISHED /* nothing more to do in phase */
> };
>
> @@ -180,7 +179,8 @@ pre_link(struct upload *p, struct sess *
> if (!sess->opts->preserve_links) {
> WARNX("%s: ignoring symlink", f->path);
> return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
> log_link(sess, f);
> return 0;
> }
> @@ -282,7 +282,8 @@ pre_dev(struct upload *p, struct sess *s
> if (!sess->opts->devices || getuid() != 0) {
> WARNX("skipping non-regular file %s", f->path);
> return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
> log_file(sess, f);
> return 0;
> }
> @@ -369,7 +370,8 @@ pre_fifo(struct upload *p, struct sess *
> if (!sess->opts->specials) {
> WARNX("skipping non-regular file %s", f->path);
> return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
> log_file(sess, f);
> return 0;
> }
> @@ -444,7 +446,8 @@ pre_sock(struct upload *p, struct sess *
> if (!sess->opts->specials) {
> WARNX("skipping non-regular file %s", f->path);
> return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
> log_file(sess, f);
> return 0;
> }
> @@ -518,7 +521,8 @@ pre_dir(const struct upload *p, struct s
> if (!sess->opts->recursive) {
> WARNX("%s: ignoring directory", f->path);
> return 0;
> - } else if (sess->opts->dry_run) {
> + }
> + if (sess->opts->dry_run) {
> log_dir(sess, f);
> return 0;
> }
> @@ -579,13 +583,14 @@ post_dir(struct sess *sess, const struct
>
> if (!sess->opts->recursive)
> return 1;
> - else if (sess->opts->dry_run)
> + if (sess->opts->dry_run)
> return 1;
>
> if (fstatat(u->rootfd, f->path, , AT_SYMLINK_NOFOLLOW) == -1) {
> ERR("%s: fstatat", f->path);
> return 0;
> - } else if (!S_ISDIR(st.st_mode)) {
> + }
> + if (!S_ISDIR(st.st_mode)) {
> WARNX("%s: not a directory", f->path);
> return 0;
> }
> @@ -631,12 +636,13 @@ post_dir(struct sess *sess, const struct
>
> /*
> * Try to open the file at the current index.
> - * If the file does not exist, returns with success.
> + * If the file does not exist, returns with >0.
> * Return <0 on failure, 0 on success w/nothing to be done, >0 on
> * success and the file needs attention.
> */
> static int
> -pre_file(const struct upload *p, int *filefd, struct sess *sess)
> +pre_file(const struct upload *p, int *filefd, struct stat *st,
> +struct sess *sess)
> {
> const struct flist *f;
>
> @@ -654,19 +660,54 @@ pre_file(const struct upload *p, int *fi
>
> /*
>* For non dry-run cases, we'll write the acknowledgement later
> - * in the rsync_uploader() function because we need to wait for
> - * the open() call to complete.
> + * in the rsync_uploader() function.
>* If the call to openat() fails with ENOENT, there's a
> - * fast-path between here and the write function, so we won't do
> - * any blocking between now and then.
> + * fast-path between here and the write function.
>*/
>
> *filefd = openat(p->rootfd, f->path,
> - O_RDONLY | O_NOFOLLOW | O_NONBLOCK, 0);
> - if (*filefd != -1 || errno == ENOENT)
> + O_RDONLY |
Re: openrsync mini cleanup
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 17:13:03 +0200:
> Normalize some code.
ok
>
> --
> :wq Claudio
>
> Index: receiver.c
> ===
> RCS file: /cvs/src/usr.bin/rsync/receiver.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 receiver.c
> --- receiver.c24 Nov 2020 16:54:44 - 1.25
> +++ receiver.c5 May 2021 15:11:38 -
> @@ -356,7 +356,7 @@ rsync_receiver(struct sess *sess, int fd
>*/
>
> if (sess->mplex_reads &&
> - (POLLIN & pfd[PFD_SENDER_IN].revents)) {
> + (pfd[PFD_SENDER_IN].revents & POLLIN)) {
> if (!io_read_flush(sess, fdin)) {
> ERRX1("io_read_flush");
> goto out;
> @@ -371,8 +371,8 @@ rsync_receiver(struct sess *sess, int fd
>* is read to mmap.
>*/
>
> - if ((POLLIN & pfd[PFD_UPLOADER_IN].revents) ||
> - (POLLOUT & pfd[PFD_SENDER_OUT].revents)) {
> + if ((pfd[PFD_UPLOADER_IN].revents & POLLIN) ||
> + (pfd[PFD_SENDER_OUT].revents & POLLOUT)) {
> c = rsync_uploader(ul,
> [PFD_UPLOADER_IN].fd,
> sess, [PFD_SENDER_OUT].fd);
> @@ -391,8 +391,8 @@ rsync_receiver(struct sess *sess, int fd
>* messages, which will otherwise clog up the pipes.
>*/
>
> - if ((POLLIN & pfd[PFD_SENDER_IN].revents) ||
> - (POLLIN & pfd[PFD_DOWNLOADER_IN].revents)) {
> + if ((pfd[PFD_SENDER_IN].revents & POLLIN) ||
> + (pfd[PFD_DOWNLOADER_IN].revents & POLLIN)) {
> c = rsync_downloader(dl, sess,
> [PFD_DOWNLOADER_IN].fd);
> if (c < 0) {
> @@ -421,10 +421,12 @@ rsync_receiver(struct sess *sess, int fd
> if (!io_write_int(sess, fdout, -1)) {
> ERRX1("io_write_int");
> goto out;
> - } else if (!io_read_int(sess, fdin, )) {
> + }
> + if (!io_read_int(sess, fdin, )) {
> ERRX1("io_read_int");
> goto out;
> - } else if (ioerror != -1) {
> + }
> + if (ioerror != -1) {
> ERRX("expected phase ack");
> goto out;
> }
> @@ -445,7 +447,8 @@ rsync_receiver(struct sess *sess, int fd
> if (!sess_stats_recv(sess, fdin)) {
> ERRX1("sess_stats_recv");
> goto out;
> - } else if (!io_write_int(sess, fdout, -1)) {
> + }
> + if (!io_write_int(sess, fdout, -1)) {
> ERRX1("io_write_int");
> goto out;
> }
>
Re: bgpd better reload behaviour
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.05.05 14:20:58 +0200:
> The peer flags (mainly rde evaluate all but also transparent-as) and the
> export options (none, default) are not properly handled on a config
> reload. In both cases a full session restart is needed after the config
> reload (with a bit of extra wait time to ensure that the peer config is
> actually up to date).
>
> The following diff should fix this.
>
> Add an extra reload barrier (IMSG_RECONF_DRAIN) to the sync of the peer
> config from the session engine to the rde. This way it can be ensured that
> the peer config is up to date in the RDE before hitting reconfiguration
> function.
>
> Store the export_type and the peer flags outside of peer->conf. Adjust all
> users of these two fields so they only look at the copies in peer.
> This way the RDE is able to notice that a value has changed during reload.
> Flush the Adj-RIB-Out for a peer where either the export_type changed or
> where rde evaluate or transparent-as is changed. After the flush the RIB
> is rebuilt in a 2nd step.
>
> Fix multiple issues in the rde_softreconfig_in_done handler that resulted
> in multiple runs of the out stage of the softreconfig pipeline.
>
> OK?
yes, ok.
> --
> :wq Claudio
>
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.519
> diff -u -p -r1.519 rde.c
> --- rde.c 27 Apr 2021 09:07:10 - 1.519
> +++ rde.c 5 May 2021 12:04:55 -
> @@ -666,6 +666,10 @@ badnetdel:
> rde_dump_ctx_throttle(imsg.hdr.pid, 1);
> }
> break;
> + case IMSG_RECONF_DRAIN:
> + imsg_compose(ibuf_se, IMSG_RECONF_DRAIN, 0, 0,
> + -1, NULL, 0);
> + break;
> default:
> break;
> }
> @@ -2136,7 +2140,7 @@ rde_update_log(const char *message, u_in
> char*p = NULL;
>
> if (!((conf->log & BGPD_LOG_UPDATES) ||
> - (peer->conf.flags & PEERFLAG_LOG_UPDATES)))
> + (peer->flags & PEERFLAG_LOG_UPDATES)))
> return;
>
> if (next != NULL)
> @@ -2919,7 +2923,7 @@ rde_generate_updates(struct rib *rib, st
> if (peer->state != PEER_UP)
> continue;
> /* handle evaluate all, keep track if it is needed */
> - if (peer->conf.flags & PEERFLAG_EVALUATE_ALL)
> + if (peer->flags & PEERFLAG_EVALUATE_ALL)
> rde_eval_all = 1;
> else if (eval_all)
> /* skip default peers if the best path didn't change */
> @@ -2931,8 +2935,8 @@ rde_generate_updates(struct rib *rib, st
> if (peer->capa.mp[aid] == 0)
> continue;
> /* skip peers with special export types */
> - if (peer->conf.export_type == EXPORT_NONE ||
> - peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
> + if (peer->export_type == EXPORT_NONE ||
> + peer->export_type == EXPORT_DEFAULT_ROUTE)
> continue;
>
> up_generate_updates(out_rules, peer, new, old);
> @@ -3289,13 +3293,34 @@ rde_reload_done(void)
> continue;
> peer->reconf_out = 0;
> peer->reconf_rib = 0;
> + if (peer->export_type != peer->conf.export_type) {
> + log_peer_info(>conf, "export type change, "
> + "reloading");
> + peer->reconf_rib = 1;
> + }
> + if ((peer->flags & PEERFLAG_EVALUATE_ALL) !=
> + (peer->conf.flags & PEERFLAG_EVALUATE_ALL)) {
> + log_peer_info(>conf, "rde evaluate change, "
> + "reloading");
> + peer->reconf_rib = 1;
> + }
> + if ((peer->flags & PEERFLAG_TRANS_AS) !=
> + (peer->conf.flags & PEERFLAG_TRANS_AS)) {
> + log_peer_info(>conf, "transparent-as change, "
> + "reloading");
> + peer->reconf_rib = 1;
> + }
> if (peer->loc_rib_id != rib_find(peer->conf.rib)) {
> log_peer_info(>conf, "rib change, reloading");
> peer->loc_rib_id = rib_find(peer->conf.rib);
> if (peer->loc_rib_id == RIB_NOTFOUND)
> fatalx("King Bula's peer met an unknown RIB");
> peer->reconf_rib = 1;
> - softreconfig++;
> + }
> + peer->export_type = peer->conf.export_type;
> + peer->flags = peer->conf.flags;
> +
> + if (peer->reconf_rib) {
> if (prefix_dump_new(peer, AID_UNSPEC,
>
ftpd(8): remove double fflush(3) calls
Hi,
The function lreply() already calls fflush(3) on stdout. So, this calls
are useless.
OK?
bye,
Jan
Index: ftpd.c
===
RCS file: /cvs/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.229
diff -u -p -r1.229 ftpd.c
--- ftpd.c 15 Jan 2020 22:06:59 - 1.229
+++ ftpd.c 5 May 2021 14:39:25 -
@@ -568,7 +568,6 @@ main(int argc, char *argv[])
line[strcspn(line, "\n")] = '\0';
lreply(530, "%s", line);
}
- (void) fflush(stdout);
(void) fclose(fp);
reply(530, "System not available.");
exit(0);
@@ -578,7 +577,6 @@ main(int argc, char *argv[])
line[strcspn(line, "\n")] = '\0';
lreply(220, "%s", line);
}
- (void) fflush(stdout);
(void) fclose(fp);
/* reply(220,) must follow */
}
@@ -1078,7 +1076,6 @@ pass(char *passwd)
line[strcspn(line, "\n")] = '\0';
lreply(230, "%s", line);
}
- (void) fflush(stdout);
(void) fclose(fp);
}
free(motd);
@@ -2029,7 +2026,6 @@ cwd(char *path)
line[strcspn(line, "\n")] = '\0';
lreply(250, "%s", line);
}
- (void) fflush(stdout);
(void) fclose(message);
}
ack("CWD");
Re: services(5): add default ftps ports
I would like a further justification for removing these ports from the very limited dynamic reserved space used by bindresvport. (but not by rresvport, which appears still stomp over them) For tcp, 32 of the 512 are locked out. For udp, 19. What software is actually using these ports? Is that software irrelevant these days? Jan Klemkow wrote: > On Wed, May 05, 2021 at 11:09:12AM +0100, Stuart Henderson wrote: > > On 2021/05/04 12:07, Jan Klemkow wrote: > > > Add missing ftps defaults ports to servies(5). > > > > > > Index: services > > > === > > > RCS file: /cvs/src/etc/services,v > > > retrieving revision 1.99 > > > diff -u -p -r1.99 services > > > --- services 18 Feb 2021 02:30:29 - 1.99 > > > +++ services 4 May 2021 10:01:35 - > > > @@ -318,6 +318,10 @@ krb_prop 754/tcp hprop # > > > Kerberos slav > > > krbupdate760/tcp kreg# BSD Kerberos > > > registration > > > supfilesrv 871/tcp # SUP server > > > swat 901/tcp # Samba Web > > > Administration Tool > > > +ftps-data989/tcp # ftp data over TLS/SSL > > > +ftps-data989/udp # ftp data over TLS/SSL > > > +ftps 990/tcp # ftp control over > > > TLS/SSL > > > +ftps 990/udp # ftp control over > > > TLS/SSL > > > > I'm OK with adding the TCP ones (though ftp-over-tls always makes me > > want to rant...). It's not going to run on UDP though so I think those > > should not be added. > > OK? > > Index: services > === > RCS file: /cvs/src/etc/services,v > retrieving revision 1.99 > diff -u -p -r1.99 services > --- services 18 Feb 2021 02:30:29 - 1.99 > +++ services 5 May 2021 12:24:29 - > @@ -318,6 +318,8 @@ krb_prop 754/tcp hprop # Kerberos slav > krbupdate760/tcp kreg# BSD Kerberos registration > supfilesrv 871/tcp # SUP server > swat 901/tcp # Samba Web Administration Tool > +ftps-data989/tcp # ftp data over TLS > +ftps 990/tcp # ftp control over TLS > supfiledbg 1127/tcp# SUP debugging > support 1529/tcp# GNATS, cygnus bug > tracker > datametrics 1645/udp >
Re: iwx(4) setkey task
Stefan Sperling writes: > On Wed, May 05, 2021 at 04:30:50PM +0200, Stefan Sperling wrote: >> This ensures that if WPA offloading is in use iwx(4) only sets link >> state UP once firmware has confirmed that crypto keys have been >> installed successfully. >> >> iwm(4) could use a similar patch, but I've not written that yet. >> >> ok? > > My previous diff had bugs, fixed here: > > Because iwx offloads both the pairwise key and the group key, depending > on when the setkey task gets scheduled this task might have to either > install one key per run or two keys in a single run. > The previous code could lead to a problem where the group key didn't > get installed, breaking broadcast/multcast traffic. > This was reported by Hrvoje who has confirmed that this version of > the patch fixes the issue. > > And obviously we should mark the link UP only once both keys have been > installed successfully. > > (The previous approach would have worked on iwm because iwm only offloads > the pairwise key.) > So far so good on my system. No regressions getting connected and using tcpbench(1) to my router that runs 6.9-stable. S3 working fine. iwx0 at pci3 dev 0 function 0 "Intel Wi-Fi 6 AX200" rev 0x1a, msix iwx0: hw rev 0x340, fw ver 48.1335886879.0, address 08:d2:3e:ee:17:8b I don't know enough of this codebase to comfortably OK, but I can say it seems to be working fine. -dv
make rsync -v less verbose
I like rsync -v but hell it is noisy with openrsync.
Just shut up about all the files that have not changed unless you go -vv.
--
:wq Claudio
Index: downloader.c
===
RCS file: /cvs/src/usr.bin/rsync/downloader.c,v
retrieving revision 1.21
diff -u -p -r1.21 downloader.c
--- downloader.c8 May 2019 21:30:11 - 1.21
+++ downloader.c5 May 2021 17:56:20 -
@@ -85,7 +85,7 @@ log_file(struct sess *sess,
if (sess->opts->server)
return;
- frac = (dl->total == 0) ? 100.0 :
+ frac = (dl->total == 0) ? 0.0 :
100.0 * dl->downloaded / dl->total;
if (dl->total > 1024 * 1024 * 1024) {
@@ -102,8 +102,12 @@ log_file(struct sess *sess,
unit = "KB";
}
- LOG1("%s (%.*f %s, %.1f%% downloaded)",
- f->path, prec, tot, unit, frac);
+ if (dl->downloaded > 0)
+ LOG1("%s (%.*f %s, %.1f%% downloaded)",
+ f->path, prec, tot, unit, frac);
+ else
+ LOG2("%s (%.*f %s, %.1f%% downloaded)",
+ f->path, prec, tot, unit, frac);
}
/*
Re: iwx(4) setkey task
On Wed, May 05, 2021 at 04:30:50PM +0200, Stefan Sperling wrote:
> This ensures that if WPA offloading is in use iwx(4) only sets link
> state UP once firmware has confirmed that crypto keys have been
> installed successfully.
>
> iwm(4) could use a similar patch, but I've not written that yet.
>
> ok?
My previous diff had bugs, fixed here:
Because iwx offloads both the pairwise key and the group key, depending
on when the setkey task gets scheduled this task might have to either
install one key per run or two keys in a single run.
The previous code could lead to a problem where the group key didn't
get installed, breaking broadcast/multcast traffic.
This was reported by Hrvoje who has confirmed that this version of
the patch fixes the issue.
And obviously we should mark the link UP only once both keys have been
installed successfully.
(The previous approach would have worked on iwm because iwm only offloads
the pairwise key.)
diff refs/heads/master refs/heads/iwx-setkey
blob - 2775f725d6347e9dbfcc2cf2ce296133ce7d6d47
blob + 40d32d22ba03d15a9a0c0bb5c015c5fecbfd3c94
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -416,6 +416,7 @@ int iwx_run_stop(struct iwx_softc *);
struct ieee80211_node *iwx_node_alloc(struct ieee80211com *);
intiwx_set_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
+void iwx_setkey_task(void *);
void iwx_delete_key(struct ieee80211com *,
struct ieee80211_node *, struct ieee80211_key *);
intiwx_media_change(struct ifnet *);
@@ -6433,6 +6434,7 @@ iwx_deauth(struct iwx_softc *sc)
}
sc->sc_flags &= ~IWX_FLAG_STA_ACTIVE;
sc->sc_rx_ba_sessions = 0;
+ in->in_flags = 0;
}
if (sc->sc_flags & IWX_FLAG_BINDING_ACTIVE) {
@@ -6498,6 +6500,7 @@ iwx_disassoc(struct iwx_softc *sc)
return err;
}
sc->sc_flags &= ~IWX_FLAG_STA_ACTIVE;
+ in->in_flags = 0;
sc->sc_rx_ba_sessions = 0;
sc->ba_start_tidmask = 0;
sc->ba_stop_tidmask = 0;
@@ -6670,13 +6673,45 @@ iwx_set_key(struct ieee80211com *ic, struct ieee80211_
struct ieee80211_key *k)
{
struct iwx_softc *sc = ic->ic_softc;
- struct iwx_add_sta_key_cmd cmd;
+ struct iwx_setkey_task_arg *a;
if (k->k_cipher != IEEE80211_CIPHER_CCMP) {
/* Fallback to software crypto for other ciphers. */
return (ieee80211_set_key(ic, ni, k));
}
+ if (sc->setkey_nkeys >= nitems(sc->setkey_arg))
+ return ENOSPC;
+
+ a = >setkey_arg[sc->setkey_cur];
+ a->sta_id = IWX_STATION_ID;
+ a->ni = ni;
+ a->k = k;
+ sc->setkey_cur = (sc->setkey_cur + 1) % nitems(sc->setkey_arg);
+ sc->setkey_nkeys++;
+ iwx_add_task(sc, systq, >setkey_task);
+ return EBUSY;
+}
+
+int
+iwx_add_sta_key(struct iwx_softc *sc, int sta_id, struct ieee80211_node *ni,
+struct ieee80211_key *k)
+{
+ struct ieee80211com *ic = >sc_ic;
+ struct iwx_node *in = (void *)ni;
+ struct iwx_add_sta_key_cmd cmd;
+ uint32_t status;
+ const int want_keymask = (IWX_NODE_FLAG_HAVE_PAIRWISE_KEY |
+ IWX_NODE_FLAG_HAVE_GROUP_KEY);
+ int err;
+
+ /*
+* Keys are stored in 'ni' so 'k' is valid if 'ni' is valid.
+* Currently we only implement station mode where 'ni' is always
+* ic->ic_bss so there is no need to validate arguments beyond this:
+*/
+ KASSERT(ni == ic->ic_bss);
+
memset(, 0, sizeof(cmd));
cmd.common.key_flags = htole16(IWX_STA_KEY_FLG_CCM |
@@ -6690,15 +6725,64 @@ iwx_set_key(struct ieee80211com *ic, struct ieee80211_
cmd.common.key_offset = 0;
memcpy(cmd.common.key, k->k_key, MIN(sizeof(cmd.common.key), k->k_len));
- cmd.common.sta_id = IWX_STATION_ID;
+ cmd.common.sta_id = sta_id;
cmd.transmit_seq_cnt = htole64(k->k_tsc);
- return iwx_send_cmd_pdu(sc, IWX_ADD_STA_KEY, IWX_CMD_ASYNC,
- sizeof(cmd), );
+ status = IWX_ADD_STA_SUCCESS;
+ err = iwx_send_cmd_pdu_status(sc, IWX_ADD_STA_KEY, sizeof(cmd), ,
+ );
+ if (sc->sc_flags & IWX_FLAG_SHUTDOWN)
+ return ECANCELED;
+ if (!err && (status & IWX_ADD_STA_STATUS_MASK) != IWX_ADD_STA_SUCCESS)
+ err = EIO;
+ if (err) {
+ IEEE80211_SEND_MGMT(ic, ni, IEEE80211_FC0_SUBTYPE_DEAUTH,
+ IEEE80211_REASON_AUTH_LEAVE);
+ ieee80211_new_state(ic, IEEE80211_S_SCAN, -1);
+ return err;
+ }
+
+ if (k->k_flags & IEEE80211_KEY_GROUP)
+ in->in_flags |= IWX_NODE_FLAG_HAVE_GROUP_KEY;
+ else
+ in->in_flags |= IWX_NODE_FLAG_HAVE_PAIRWISE_KEY;
+
+ if ((in->in_flags & want_keymask) == want_keymask) {
+ DPRINTF(("marking
Re: rpki-client: change "asn" from string to integer in JSON output
Makes sense to me.
Job Snijders wrote:
> I'd like to modify our JSON format, many people in the community have
> voiced complaints that transforming the string to an integer is
> annoying.
>
> This won't break existing deployments coupled with GoRTR.
>
> OK?
>
> Index: output-json.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 output-json.c
> --- output-json.c 8 Apr 2021 19:49:27 - 1.15
> +++ output-json.c 5 May 2021 15:29:15 -
> @@ -100,7 +100,7 @@ output_json(FILE *out, struct vrp_tree *
>
> ip_addr_print(>addr, v->afi, buf, sizeof(buf));
>
> - if (fprintf(out, "\t\t{ \"asn\": \"AS%u\", \"prefix\": \"%s\", "
> + if (fprintf(out, "\t\t{ \"asn\": %u, \"prefix\": \"%s\", "
> "\"maxLength\": %u, \"ta\": \"%s\" }",
> v->asid, buf, v->maxlength, v->tal) < 0)
> return -1;
>
rpki-client: change "asn" from string to integer in JSON output
I'd like to modify our JSON format, many people in the community have
voiced complaints that transforming the string to an integer is
annoying.
This won't break existing deployments coupled with GoRTR.
OK?
Index: output-json.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/output-json.c,v
retrieving revision 1.15
diff -u -p -r1.15 output-json.c
--- output-json.c 8 Apr 2021 19:49:27 - 1.15
+++ output-json.c 5 May 2021 15:29:15 -
@@ -100,7 +100,7 @@ output_json(FILE *out, struct vrp_tree *
ip_addr_print(>addr, v->afi, buf, sizeof(buf));
- if (fprintf(out, "\t\t{ \"asn\": \"AS%u\", \"prefix\": \"%s\", "
+ if (fprintf(out, "\t\t{ \"asn\": %u, \"prefix\": \"%s\", "
"\"maxLength\": %u, \"ta\": \"%s\" }",
v->asid, buf, v->maxlength, v->tal) < 0)
return -1;
Re: services(5): add default ftps ports
On Wed, May 05, 2021 at 11:09:12AM +0100, Stuart Henderson wrote: > On 2021/05/04 12:07, Jan Klemkow wrote: > > Add missing ftps defaults ports to servies(5). > > > > Index: services > > === > > RCS file: /cvs/src/etc/services,v > > retrieving revision 1.99 > > diff -u -p -r1.99 services > > --- services18 Feb 2021 02:30:29 - 1.99 > > +++ services4 May 2021 10:01:35 - > > @@ -318,6 +318,10 @@ krb_prop 754/tcp hprop # > > Kerberos slav > > krbupdate 760/tcp kreg# BSD Kerberos registration > > supfilesrv 871/tcp # SUP server > > swat 901/tcp # Samba Web > > Administration Tool > > +ftps-data 989/tcp # ftp data over TLS/SSL > > +ftps-data 989/udp # ftp data over TLS/SSL > > +ftps 990/tcp # ftp control over > > TLS/SSL > > +ftps 990/udp # ftp control over > > TLS/SSL > > I'm OK with adding the TCP ones (though ftp-over-tls always makes me > want to rant...). It's not going to run on UDP though so I think those > should not be added. OK? Index: services === RCS file: /cvs/src/etc/services,v retrieving revision 1.99 diff -u -p -r1.99 services --- services18 Feb 2021 02:30:29 - 1.99 +++ services5 May 2021 12:24:29 - @@ -318,6 +318,8 @@ krb_prop754/tcp hprop # Kerberos slav krbupdate 760/tcp kreg# BSD Kerberos registration supfilesrv 871/tcp # SUP server swat 901/tcp # Samba Web Administration Tool +ftps-data 989/tcp # ftp data over TLS +ftps 990/tcp # ftp control over TLS supfiledbg 1127/tcp# SUP debugging support1529/tcp# GNATS, cygnus bug tracker datametrics1645/udp
btrace: fflush(stdout) after printf
Hi,
Thanks for the wonderful work on dt(4) and the btrace tool.
I noticed while using the btrace program:
btrace -e 'tracepoint:uvm:malloc { printf("%s[%d]: malloc(%d)\n", comm, pid,
arg0); }' | \
grep -v Xorg
The output of btrace is unbuffered and grep does not filter the line, until the
buffer is full of course.
The below patch flushes stdout after a printf call, does it make sense?
diff --git usr.sbin/btrace/printf.c usr.sbin/btrace/printf.c
index d7a9424dce6..89f464bc551 100644
--- usr.sbin/btrace/printf.c
+++ usr.sbin/btrace/printf.c
@@ -220,6 +220,8 @@ stmt_printf(struct bt_stmt *bs, struct dt_evt *des)
}
} while (gargv != NULL);
+ fflush(stdout);
+
return (rval);
}
--
Kind regards,
Hiltjo
simplify the openrsync uploader
The rsync uploader (what is the generator in rsync) can be simplified and
cleaned up a fair bit.
There is some confusion of non-blocking IO on regular files and the idea
to poll() between openat() and fstat(). This is all not needed and
therefor a lot of the code handling files can be moved into pre_file.
This also removes the UPLOAD_READ_LOCAL state since it is no longer
needed.
As a little extra gift this diff also plugs a mem leak in rsync_uploader.
The mbuf buffer was not freed and so for every file being checksummed it
would leak one of those.
--
:wq Claudio
Index: uploader.c
===
RCS file: /cvs/src/usr.bin/rsync/uploader.c,v
retrieving revision 1.24
diff -u -p -r1.24 uploader.c
--- uploader.c 22 Mar 2021 11:20:04 - 1.24
+++ uploader.c 5 May 2021 15:39:10 -
@@ -33,8 +33,7 @@
enum uploadst {
UPLOAD_FIND_NEXT = 0, /* find next to upload to sender */
- UPLOAD_WRITE_LOCAL, /* wait to write to sender */
- UPLOAD_READ_LOCAL, /* wait to read from local file */
+ UPLOAD_WRITE, /* wait to write to sender */
UPLOAD_FINISHED /* nothing more to do in phase */
};
@@ -180,7 +179,8 @@ pre_link(struct upload *p, struct sess *
if (!sess->opts->preserve_links) {
WARNX("%s: ignoring symlink", f->path);
return 0;
- } else if (sess->opts->dry_run) {
+ }
+ if (sess->opts->dry_run) {
log_link(sess, f);
return 0;
}
@@ -282,7 +282,8 @@ pre_dev(struct upload *p, struct sess *s
if (!sess->opts->devices || getuid() != 0) {
WARNX("skipping non-regular file %s", f->path);
return 0;
- } else if (sess->opts->dry_run) {
+ }
+ if (sess->opts->dry_run) {
log_file(sess, f);
return 0;
}
@@ -369,7 +370,8 @@ pre_fifo(struct upload *p, struct sess *
if (!sess->opts->specials) {
WARNX("skipping non-regular file %s", f->path);
return 0;
- } else if (sess->opts->dry_run) {
+ }
+ if (sess->opts->dry_run) {
log_file(sess, f);
return 0;
}
@@ -444,7 +446,8 @@ pre_sock(struct upload *p, struct sess *
if (!sess->opts->specials) {
WARNX("skipping non-regular file %s", f->path);
return 0;
- } else if (sess->opts->dry_run) {
+ }
+ if (sess->opts->dry_run) {
log_file(sess, f);
return 0;
}
@@ -518,7 +521,8 @@ pre_dir(const struct upload *p, struct s
if (!sess->opts->recursive) {
WARNX("%s: ignoring directory", f->path);
return 0;
- } else if (sess->opts->dry_run) {
+ }
+ if (sess->opts->dry_run) {
log_dir(sess, f);
return 0;
}
@@ -579,13 +583,14 @@ post_dir(struct sess *sess, const struct
if (!sess->opts->recursive)
return 1;
- else if (sess->opts->dry_run)
+ if (sess->opts->dry_run)
return 1;
if (fstatat(u->rootfd, f->path, , AT_SYMLINK_NOFOLLOW) == -1) {
ERR("%s: fstatat", f->path);
return 0;
- } else if (!S_ISDIR(st.st_mode)) {
+ }
+ if (!S_ISDIR(st.st_mode)) {
WARNX("%s: not a directory", f->path);
return 0;
}
@@ -631,12 +636,13 @@ post_dir(struct sess *sess, const struct
/*
* Try to open the file at the current index.
- * If the file does not exist, returns with success.
+ * If the file does not exist, returns with >0.
* Return <0 on failure, 0 on success w/nothing to be done, >0 on
* success and the file needs attention.
*/
static int
-pre_file(const struct upload *p, int *filefd, struct sess *sess)
+pre_file(const struct upload *p, int *filefd, struct stat *st,
+struct sess *sess)
{
const struct flist *f;
@@ -654,19 +660,54 @@ pre_file(const struct upload *p, int *fi
/*
* For non dry-run cases, we'll write the acknowledgement later
-* in the rsync_uploader() function because we need to wait for
-* the open() call to complete.
+* in the rsync_uploader() function.
* If the call to openat() fails with ENOENT, there's a
-* fast-path between here and the write function, so we won't do
-* any blocking between now and then.
+* fast-path between here and the write function.
*/
*filefd = openat(p->rootfd, f->path,
- O_RDONLY | O_NOFOLLOW | O_NONBLOCK, 0);
- if (*filefd != -1 || errno == ENOENT)
+ O_RDONLY | O_NOFOLLOW, 0);
+
+ if (*filefd == -1 && errno != ENOENT) {
+ ERR("%s: openat", f->path);
+ return -1;
+ }
+ if (*filefd == -1)
return 1;
- ERR("%s: openat", f->path);
-
openrsync mini cleanup
Normalize some code.
--
:wq Claudio
Index: receiver.c
===
RCS file: /cvs/src/usr.bin/rsync/receiver.c,v
retrieving revision 1.25
diff -u -p -r1.25 receiver.c
--- receiver.c 24 Nov 2020 16:54:44 - 1.25
+++ receiver.c 5 May 2021 15:11:38 -
@@ -356,7 +356,7 @@ rsync_receiver(struct sess *sess, int fd
*/
if (sess->mplex_reads &&
- (POLLIN & pfd[PFD_SENDER_IN].revents)) {
+ (pfd[PFD_SENDER_IN].revents & POLLIN)) {
if (!io_read_flush(sess, fdin)) {
ERRX1("io_read_flush");
goto out;
@@ -371,8 +371,8 @@ rsync_receiver(struct sess *sess, int fd
* is read to mmap.
*/
- if ((POLLIN & pfd[PFD_UPLOADER_IN].revents) ||
- (POLLOUT & pfd[PFD_SENDER_OUT].revents)) {
+ if ((pfd[PFD_UPLOADER_IN].revents & POLLIN) ||
+ (pfd[PFD_SENDER_OUT].revents & POLLOUT)) {
c = rsync_uploader(ul,
[PFD_UPLOADER_IN].fd,
sess, [PFD_SENDER_OUT].fd);
@@ -391,8 +391,8 @@ rsync_receiver(struct sess *sess, int fd
* messages, which will otherwise clog up the pipes.
*/
- if ((POLLIN & pfd[PFD_SENDER_IN].revents) ||
- (POLLIN & pfd[PFD_DOWNLOADER_IN].revents)) {
+ if ((pfd[PFD_SENDER_IN].revents & POLLIN) ||
+ (pfd[PFD_DOWNLOADER_IN].revents & POLLIN)) {
c = rsync_downloader(dl, sess,
[PFD_DOWNLOADER_IN].fd);
if (c < 0) {
@@ -421,10 +421,12 @@ rsync_receiver(struct sess *sess, int fd
if (!io_write_int(sess, fdout, -1)) {
ERRX1("io_write_int");
goto out;
- } else if (!io_read_int(sess, fdin, )) {
+ }
+ if (!io_read_int(sess, fdin, )) {
ERRX1("io_read_int");
goto out;
- } else if (ioerror != -1) {
+ }
+ if (ioerror != -1) {
ERRX("expected phase ack");
goto out;
}
@@ -445,7 +447,8 @@ rsync_receiver(struct sess *sess, int fd
if (!sess_stats_recv(sess, fdin)) {
ERRX1("sess_stats_recv");
goto out;
- } else if (!io_write_int(sess, fdout, -1)) {
+ }
+ if (!io_write_int(sess, fdout, -1)) {
ERRX1("io_write_int");
goto out;
}
iwx(4) setkey task
This ensures that if WPA offloading is in use iwx(4) only sets link
state UP once firmware has confirmed that crypto keys have been
installed successfully.
iwm(4) could use a similar patch, but I've not written that yet.
ok?
diff f30a2d1c29071de858b3607fbaa2227aa7c6bfd3
57e0ba6871dcbf4d5516ac085c5c82b830b96e3d
blob - 2775f725d6347e9dbfcc2cf2ce296133ce7d6d47
blob + c0303528fdbc71291f3d5bf4d2677d351c86eb9c
--- sys/dev/pci/if_iwx.c
+++ sys/dev/pci/if_iwx.c
@@ -416,6 +416,7 @@ int iwx_run_stop(struct iwx_softc *);
struct ieee80211_node *iwx_node_alloc(struct ieee80211com *);
intiwx_set_key(struct ieee80211com *, struct ieee80211_node *,
struct ieee80211_key *);
+void iwx_setkey_task(void *);
void iwx_delete_key(struct ieee80211com *,
struct ieee80211_node *, struct ieee80211_key *);
intiwx_media_change(struct ifnet *);
@@ -6670,13 +6671,56 @@ iwx_set_key(struct ieee80211com *ic, struct ieee80211_
struct ieee80211_key *k)
{
struct iwx_softc *sc = ic->ic_softc;
- struct iwx_add_sta_key_cmd cmd;
+ struct iwx_setkey_task_arg *a;
if (k->k_cipher != IEEE80211_CIPHER_CCMP) {
/* Fallback to software crypto for other ciphers. */
return (ieee80211_set_key(ic, ni, k));
}
+ a = >setkey_arg[sc->setkey_cur];
+ if (a->ni != NULL)
+ return ENOSPC;
+ a->sta_id = IWX_STATION_ID;
+ a->ni = ni;
+ a->k = k;
+ sc->setkey_cur = (sc->setkey_cur + 1) % nitems(sc->setkey_arg);
+
+ iwx_add_task(sc, systq, >setkey_task);
+ return EBUSY;
+}
+
+void
+iwx_setkey_task(void *arg)
+{
+ struct iwx_softc *sc = arg;
+ struct iwx_setkey_task_arg *a;
+ struct ieee80211com *ic = >sc_ic;
+ struct ieee80211_node *ni;
+ struct ieee80211_key *k;
+ int sta_id;
+ struct iwx_add_sta_key_cmd cmd;
+ uint32_t status;
+ int err, s = splnet();
+
+ if (sc->sc_flags & IWX_FLAG_SHUTDOWN)
+ goto out;
+
+ a = >setkey_arg[sc->setkey_tail];
+ sta_id = a->sta_id;
+ ni = a->ni;
+ a->ni = NULL;
+ k = a->k;
+ a->k = NULL;
+ sc->setkey_tail = (sc->setkey_tail + 1) % nitems(sc->setkey_arg);
+
+ /*
+* Keys are stored in 'ni' so 'k' is valid if 'ni' is valid.
+* Currently we only implement station mode where 'ni' is always
+* ic->ic_bss so there is no need to validate arguments beyond this:
+*/
+ KASSERT(ni == ic->ic_bss);
+
memset(, 0, sizeof(cmd));
cmd.common.key_flags = htole16(IWX_STA_KEY_FLG_CCM |
@@ -6690,12 +6734,29 @@ iwx_set_key(struct ieee80211com *ic, struct ieee80211_
cmd.common.key_offset = 0;
memcpy(cmd.common.key, k->k_key, MIN(sizeof(cmd.common.key), k->k_len));
- cmd.common.sta_id = IWX_STATION_ID;
+ cmd.common.sta_id = sta_id;
cmd.transmit_seq_cnt = htole64(k->k_tsc);
- return iwx_send_cmd_pdu(sc, IWX_ADD_STA_KEY, IWX_CMD_ASYNC,
- sizeof(cmd), );
+ status = IWX_ADD_STA_SUCCESS;
+ err = iwx_send_cmd_pdu_status(sc, IWX_ADD_STA_KEY, sizeof(cmd), ,
+ );
+ if (sc->sc_flags & IWX_FLAG_SHUTDOWN)
+ goto out;
+ if (err || (status & IWX_ADD_STA_STATUS_MASK) != IWX_ADD_STA_SUCCESS) {
+ IEEE80211_SEND_MGMT(ic, ni, IEEE80211_FC0_SUBTYPE_DEAUTH,
+ IEEE80211_REASON_AUTH_LEAVE);
+ ieee80211_new_state(ic, IEEE80211_S_SCAN, -1);
+ } else {
+ DPRINTF(("marking port %s valid\n",
+ ether_sprintf(ni->ni_macaddr)));
+ ni->ni_port_valid = 1;
+ ieee80211_set_link_state(ic, LINK_STATE_UP);
+ }
+out:
+ refcnt_rele_wake(>task_refs);
+ splx(s);
+ return;
}
void
@@ -6868,6 +6929,9 @@ iwx_newstate(struct ieee80211com *ic, enum ieee80211_s
if (ic->ic_state == IEEE80211_S_RUN) {
iwx_del_task(sc, systq, >ba_task);
+ iwx_del_task(sc, systq, >setkey_task);
+ memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg));
+ sc->setkey_cur = sc->setkey_tail = 0;
iwx_del_task(sc, systq, >mac_ctxt_task);
for (i = 0; i < nitems(sc->sc_rxba_data); i++) {
struct iwx_rxba_data *rxba = >sc_rxba_data[i];
@@ -7440,6 +7504,9 @@ iwx_stop(struct ifnet *ifp)
task_del(systq, >init_task);
iwx_del_task(sc, sc->sc_nswq, >newstate_task);
iwx_del_task(sc, systq, >ba_task);
+ iwx_del_task(sc, systq, >setkey_task);
+ memset(sc->setkey_arg, 0, sizeof(sc->setkey_arg));
+ sc->setkey_cur = sc->setkey_tail = 0;
iwx_del_task(sc, systq, >mac_ctxt_task);
KASSERT(sc->task_refs.refs >= 1);
refcnt_finalize(>task_refs, "iwxstop");
@@ -8837,6 +8904,7 @@ iwx_attach(struct device *parent, struct device *self,
task_set(>init_task, iwx_init_task,
Re: unlock lseek(2)
On Sat, May 01, 2021 at 08:19:19AM +0200, Anton Lindqvist wrote:
> Hi,
> In August 2019 I tried to unlock lseek(2) which failed since the vnode
> lock could not be acquired without holding the kernel lock back then,
> found the hard way. claudio@ recently[1] make it possible to acquire a
> vnode lock without holding the kernel lock. I therefore would like to
> give this another try. The kernel lock is still required around
> VOP_GETATTR() as the underlying file system implementations are not
> yet^W MP-safe.
>
> Comments? OK?
>
> [1]
> https://github.com/openbsd/src/commit/9d6122f62b6ed32d6c956e1d5269114b2f24ea14
>
> Index: kern/syscalls.master
> ===
> RCS file: /cvs/src/sys/kern/syscalls.master,v
> retrieving revision 1.209
> diff -u -p -r1.209 syscalls.master
> --- kern/syscalls.master 18 Mar 2021 08:43:38 - 1.209
> +++ kern/syscalls.master 29 Apr 2021 19:09:58 -
> @@ -349,7 +349,7 @@
> 197 STD { void *sys_mmap(void *addr, size_t len, int prot, \
> int flags, int fd, long pad, off_t pos); }
> 198 INDIR { quad_t sys___syscall(quad_t num, ...); }
> -199 STD { off_t sys_lseek(int fd, int pad, off_t offset, \
> +199 STD NOLOCK { off_t sys_lseek(int fd, int pad, off_t offset, \
> int whence); }
> 200 STD { int sys_truncate(const char *path, int pad, \
> off_t length); }
> Index: kern/vfs_vnops.c
> ===
> RCS file: /cvs/src/sys/kern/vfs_vnops.c,v
> retrieving revision 1.115
> diff -u -p -r1.115 vfs_vnops.c
> --- kern/vfs_vnops.c 28 Apr 2021 09:53:53 - 1.115
> +++ kern/vfs_vnops.c 29 Apr 2021 19:09:58 -
> @@ -657,7 +657,9 @@ vn_seek(struct file *fp, off_t *offset,
> newoff = fp->f_offset + *offset;
> break;
> case SEEK_END:
> + KERNEL_LOCK();
> error = VOP_GETATTR(vp, , cred, p);
> + KERNEL_UNLOCK();
> if (error)
> goto out;
> newoff = *offset + (off_t)vattr.va_size;
>
Been running with this for a while. Both make build and a regress run
done.
Diff make sense. OK claudio@
--
:wq Claudio
bgpd better reload behaviour
The peer flags (mainly rde evaluate all but also transparent-as) and the
export options (none, default) are not properly handled on a config
reload. In both cases a full session restart is needed after the config
reload (with a bit of extra wait time to ensure that the peer config is
actually up to date).
The following diff should fix this.
Add an extra reload barrier (IMSG_RECONF_DRAIN) to the sync of the peer
config from the session engine to the rde. This way it can be ensured that
the peer config is up to date in the RDE before hitting reconfiguration
function.
Store the export_type and the peer flags outside of peer->conf. Adjust all
users of these two fields so they only look at the copies in peer.
This way the RDE is able to notice that a value has changed during reload.
Flush the Adj-RIB-Out for a peer where either the export_type changed or
where rde evaluate or transparent-as is changed. After the flush the RIB
is rebuilt in a 2nd step.
Fix multiple issues in the rde_softreconfig_in_done handler that resulted
in multiple runs of the out stage of the softreconfig pipeline.
OK?
--
:wq Claudio
Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.519
diff -u -p -r1.519 rde.c
--- rde.c 27 Apr 2021 09:07:10 - 1.519
+++ rde.c 5 May 2021 12:04:55 -
@@ -666,6 +666,10 @@ badnetdel:
rde_dump_ctx_throttle(imsg.hdr.pid, 1);
}
break;
+ case IMSG_RECONF_DRAIN:
+ imsg_compose(ibuf_se, IMSG_RECONF_DRAIN, 0, 0,
+ -1, NULL, 0);
+ break;
default:
break;
}
@@ -2136,7 +2140,7 @@ rde_update_log(const char *message, u_in
char*p = NULL;
if (!((conf->log & BGPD_LOG_UPDATES) ||
- (peer->conf.flags & PEERFLAG_LOG_UPDATES)))
+ (peer->flags & PEERFLAG_LOG_UPDATES)))
return;
if (next != NULL)
@@ -2919,7 +2923,7 @@ rde_generate_updates(struct rib *rib, st
if (peer->state != PEER_UP)
continue;
/* handle evaluate all, keep track if it is needed */
- if (peer->conf.flags & PEERFLAG_EVALUATE_ALL)
+ if (peer->flags & PEERFLAG_EVALUATE_ALL)
rde_eval_all = 1;
else if (eval_all)
/* skip default peers if the best path didn't change */
@@ -2931,8 +2935,8 @@ rde_generate_updates(struct rib *rib, st
if (peer->capa.mp[aid] == 0)
continue;
/* skip peers with special export types */
- if (peer->conf.export_type == EXPORT_NONE ||
- peer->conf.export_type == EXPORT_DEFAULT_ROUTE)
+ if (peer->export_type == EXPORT_NONE ||
+ peer->export_type == EXPORT_DEFAULT_ROUTE)
continue;
up_generate_updates(out_rules, peer, new, old);
@@ -3289,13 +3293,34 @@ rde_reload_done(void)
continue;
peer->reconf_out = 0;
peer->reconf_rib = 0;
+ if (peer->export_type != peer->conf.export_type) {
+ log_peer_info(>conf, "export type change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
+ if ((peer->flags & PEERFLAG_EVALUATE_ALL) !=
+ (peer->conf.flags & PEERFLAG_EVALUATE_ALL)) {
+ log_peer_info(>conf, "rde evaluate change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
+ if ((peer->flags & PEERFLAG_TRANS_AS) !=
+ (peer->conf.flags & PEERFLAG_TRANS_AS)) {
+ log_peer_info(>conf, "transparent-as change, "
+ "reloading");
+ peer->reconf_rib = 1;
+ }
if (peer->loc_rib_id != rib_find(peer->conf.rib)) {
log_peer_info(>conf, "rib change, reloading");
peer->loc_rib_id = rib_find(peer->conf.rib);
if (peer->loc_rib_id == RIB_NOTFOUND)
fatalx("King Bula's peer met an unknown RIB");
peer->reconf_rib = 1;
- softreconfig++;
+ }
+ peer->export_type = peer->conf.export_type;
+ peer->flags = peer->conf.flags;
+
+ if (peer->reconf_rib) {
if (prefix_dump_new(peer, AID_UNSPEC,
RDE_RUNNER_ROUNDS, NULL, rde_up_flush_upcall,
rde_softreconfig_in_done, NULL) == -1)
@@ -3362,15 +3387,15 @@
Re: services(5): add default ftps ports
Stuart Henderson wrote: > Every new entry in this file reduces the range available for dynamic > port selection, so it would seem a good idea to cull a few if we're > adding some. Here are some likely candidates; Precisely. And one day there will be no reserved ports left, and then what?
Re: services(5): add default ftps ports
reads good. OK florian On 2021-05-05 11:09 +01, Stuart Henderson wrote: > On 2021/05/04 12:07, Jan Klemkow wrote: >> Hi, >> >> Add missing ftps defaults ports to servies(5). >> >> OK? >> >> bye, >> Jan >> >> Index: services >> === >> RCS file: /cvs/src/etc/services,v >> retrieving revision 1.99 >> diff -u -p -r1.99 services >> --- services 18 Feb 2021 02:30:29 - 1.99 >> +++ services 4 May 2021 10:01:35 - >> @@ -318,6 +318,10 @@ krb_prop754/tcp hprop # >> Kerberos slav >> krbupdate 760/tcp kreg# BSD Kerberos registration >> supfilesrv 871/tcp # SUP server >> swat901/tcp # Samba Web >> Administration Tool >> +ftps-data 989/tcp # ftp data over TLS/SSL >> +ftps-data 989/udp # ftp data over TLS/SSL >> +ftps990/tcp # ftp control over >> TLS/SSL >> +ftps990/udp # ftp control over >> TLS/SSL > > I'm OK with adding the TCP ones (though ftp-over-tls always makes me > want to rant...). It's not going to run on UDP though so I think those > should not be added. > > Every new entry in this file reduces the range available for dynamic > port selection, so it would seem a good idea to cull a few if we're > adding some. Here are some likely candidates; > > - removed a few UDP entries for protocols that won't use it > > - dropped some obsolete protocols > > - moved smtps/465 to the standards section (rfc8314) > > - moved the IANA UDP/TCP policy from a comment in /etc/services to > the manual, and added a pointer to the baddynamic sysctls > > Index: share/man/man5/services.5 > === > RCS file: /cvs/src/share/man/man5/services.5,v > retrieving revision 1.13 > diff -u -p -r1.13 services.5 > --- share/man/man5/services.5 3 Mar 2019 17:04:17 - 1.13 > +++ share/man/man5/services.5 5 May 2021 09:56:49 - > @@ -63,6 +63,20 @@ end of the line are not interpreted by t > .Pp > Service names may contain any printable character other than a > field delimiter, newline, or comment character. > +.Pp > +To protect service ports from being used for dynamic port assignment, > +.Xr rc 8 > +reads > +.Nm > +at boot and uses the contents to populate > +.Va net.inet.tcp.baddynamic > +and > +.Va net.inet.udp.baddynamic . > +.Pp > +While it is the policy of IANA to assign a single well-known port number > +for both TCP and UDP, to avoid reducing the dynamic port range unnecessarily, > +the unused entries are not always listed in > +.Nm . > .Sh FILES > .Bl -tag -width /etc/services -compact > .It Pa /etc/services > Index: etc/services > === > RCS file: /cvs/src/etc/services,v > retrieving revision 1.99 > diff -u -p -r1.99 services > --- etc/services 18 Feb 2021 02:30:29 - 1.99 > +++ etc/services 5 May 2021 09:56:49 - > @@ -3,10 +3,6 @@ > # Network services, Internet style > # > https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt > # > -# Note that it is presently the policy of IANA to assign a single well-known > -# port number for both TCP and UDP; hence, most entries here have two entries > -# even if the protocol doesn't support UDP operations. > -# > > tcpmux 1/tcp # TCP port service > multiplexer > echo 7/tcp > @@ -64,10 +60,7 @@ csnet-ns 105/tcp cso-ns # also used by > csnet-ns 105/udp cso-ns > rtelnet 107/tcp # Remote Telnet > rtelnet 107/udp > -pop2 109/tcp postoffice # POP version 2 > -pop2 109/udp > pop3 110/tcp # POP version 3 > -pop3 110/udp > sunrpc 111/tcp portmap rpcbind > sunrpc 111/udp portmap rpcbind > auth 113/tcp authentication tap ident > @@ -87,7 +80,6 @@ netbios-dgm 138/udp > netbios-ssn 139/tcp # NETBIOS session service > netbios-ssn 139/udp > imap 143/tcp imap2 # Internet Message Access Proto > -imap 143/udp imap2 # Internet Message Access Proto > bftp 152/tcp # Background File Transfer Proto > snmp 161/udp # Simple Net Mgmt Proto > snmp-trap162/udp snmptrap# Traps for SNMP > @@ -100,11 +92,9 @@ xdmcp 177/udp > nextstep 178/tcp NeXTStep NextStep # NeXTStep window > nextstep 178/udp NeXTStep NextStep # server > bgp 179/tcp # Border Gateway Proto. > -bgp 179/udp >
Re: ld.so: NULL dereference on corrupt library
On 04/05/21(Tue) 21:41, Klemens Nanni wrote:
> On Thu, Apr 15, 2021 at 03:05:45PM +0200, Mark Kettenis wrote:
> > > > [...]
> > > > Hence, only access buckets in _dl_find_symbol_obj() if there are any;
> > > > this fixes the crash and in fact allows me to play the song even when
> > > > preloading the currupted library, i.e.
> > > >
> > > > $ LD_PRELOAD=./libvorbisenc.so.3.1 ogg123 song62.ogg
> > > >
> > > > now also works with patched ld.so installed -- I'd expected ld.so,
> > > > libvorbis or ogg123 to crash on some other place...
> > > >
> > > > I'm not sure what to make of this, I also don't know enough about ld.so
> > > > to judge this diff in context, it does however fix an obvious error.
> > > > FWIW, regress/libexec/ld.so runs fine with this diff.
> > >
> > > I'm not sure if silently ignoring the corruption is the best way to go.
> >
> > It certainly isn't. If corruption is detected, the prcess should
> > terminate immedtaley.
>
> I totally agree, mention of regress passing shouldn't imply that I want
> that diff in, but rather show that it didn't have unexpected drawbacks.
>
> > > Do you know why `nbuckets' and `buckets_elf' aren't initialized for this
> > > object? Do you know if _dl_finalize_object() has been call for it?
>
> Yes, _dl_finalize_object() is always called for it.
>
> I compared my corrupted shared library with an intact copy from ports
> and it showed that the corrupted one was simply zeroed out at some point
> (not truncated) until the end, e.g. readelf(1)'s `-h' or `-l' report
> "Error: no .dynamic section in the dynamic segment".
>
> So this isn't a case of some badly linked library or one that has a few
> bits flipped, it's simply a partial one... seems like bad luck?
>
> > > > Is this a code path that can happen with intact objects?
> > > > Given that the file is obviously corrupted but programs using it still
> > > > (partially) work, should a warning be printed in this case?
> > >
> > > Indicating that the library is corrupted might indeed be better than
> > > crashing. However it isn't clear to me where such check should happen.
>
> I've done just that now as there's nothing else to do. It is an obscure
> case that I cannot explain without corruption, so very unlikely to
> happen, but now it did...
>
> > > > Index: resolve.c
> > > > ===
> > > > RCS file: /cvs/src/libexec/ld.so/resolve.c,v
> > > > retrieving revision 1.94
> > > > diff -u -p -r1.94 resolve.c
> > > > --- resolve.c 4 Oct 2019 17:42:16 - 1.94
> > > > +++ resolve.c 14 Apr 2021 15:56:14 -
> > > > @@ -608,7 +608,7 @@ _dl_find_symbol_obj(elf_object_t *obj, s
> > > > return r > 0;
> > > > }
> > > > } while ((*hashval++ & 1U) == 0);
> > > > - } else {
> > > > + } else if (obj->nbuckets > 0) {
> > > > Elf_Word si;
> > > >
> > > > for (si = obj->buckets_elf[sl->sl_elf_hash %
> > > > obj->nbuckets];
> > > >
> > >
> > >
>
> readelf(1) detects this corruption as missing (or empty/zeroed out as
> code reading showed); we could probably to that as well but it'd be
> less trivial and a generalisation of my issue.
>
> So the new diff simply bails out if there is no symbol hash table, which
> is equally relevant for both ELF and GNU hashes:
>
> $ LD_PRELOAD=./bad.so ./ogg123 ~/song62.ogg
> ld.so: ogg123.test: ./bad.so: no buckets
> killed
> $
>
> Feedback? Objections? OK?
I still don't understand what the corruption is and the check below
doesn't explain that either. So if I'm developing a library and I see
such message it doesn't give me more info than the previous core dump.
What is corrupted? The header? A section is missing? An offset is
incorrect? Is there a mismatch between DT_GNU_HASH and DT_HASH?
> Index: resolve.c
> ===
> RCS file: /cvs/src/libexec/ld.so/resolve.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 resolve.c
> --- resolve.c 4 Oct 2019 17:42:16 - 1.94
> +++ resolve.c 29 Apr 2021 22:07:46 -
> @@ -573,6 +573,9 @@ _dl_find_symbol_obj(elf_object_t *obj, s
> {
> const Elf_Sym *symt = obj->dyn.symtab;
>
> + if (obj->nbuckets == 0)
> + _dl_die("%s: no buckets", obj->load_name);
> +
> if (obj->status & STAT_GNU_HASH) {
> uint32_t hash = sl->sl_gnu_hash;
> Elf_Addr bloom_word;
Re: services(5): add default ftps ports
On 2021/05/04 12:07, Jan Klemkow wrote: > Hi, > > Add missing ftps defaults ports to servies(5). > > OK? > > bye, > Jan > > Index: services > === > RCS file: /cvs/src/etc/services,v > retrieving revision 1.99 > diff -u -p -r1.99 services > --- services 18 Feb 2021 02:30:29 - 1.99 > +++ services 4 May 2021 10:01:35 - > @@ -318,6 +318,10 @@ krb_prop 754/tcp hprop # Kerberos slav > krbupdate760/tcp kreg# BSD Kerberos registration > supfilesrv 871/tcp # SUP server > swat 901/tcp # Samba Web Administration Tool > +ftps-data989/tcp # ftp data over TLS/SSL > +ftps-data989/udp # ftp data over TLS/SSL > +ftps 990/tcp # ftp control over TLS/SSL > +ftps 990/udp # ftp control over TLS/SSL I'm OK with adding the TCP ones (though ftp-over-tls always makes me want to rant...). It's not going to run on UDP though so I think those should not be added. Every new entry in this file reduces the range available for dynamic port selection, so it would seem a good idea to cull a few if we're adding some. Here are some likely candidates; - removed a few UDP entries for protocols that won't use it - dropped some obsolete protocols - moved smtps/465 to the standards section (rfc8314) - moved the IANA UDP/TCP policy from a comment in /etc/services to the manual, and added a pointer to the baddynamic sysctls Index: share/man/man5/services.5 === RCS file: /cvs/src/share/man/man5/services.5,v retrieving revision 1.13 diff -u -p -r1.13 services.5 --- share/man/man5/services.5 3 Mar 2019 17:04:17 - 1.13 +++ share/man/man5/services.5 5 May 2021 09:56:49 - @@ -63,6 +63,20 @@ end of the line are not interpreted by t .Pp Service names may contain any printable character other than a field delimiter, newline, or comment character. +.Pp +To protect service ports from being used for dynamic port assignment, +.Xr rc 8 +reads +.Nm +at boot and uses the contents to populate +.Va net.inet.tcp.baddynamic +and +.Va net.inet.udp.baddynamic . +.Pp +While it is the policy of IANA to assign a single well-known port number +for both TCP and UDP, to avoid reducing the dynamic port range unnecessarily, +the unused entries are not always listed in +.Nm . .Sh FILES .Bl -tag -width /etc/services -compact .It Pa /etc/services Index: etc/services === RCS file: /cvs/src/etc/services,v retrieving revision 1.99 diff -u -p -r1.99 services --- etc/services18 Feb 2021 02:30:29 - 1.99 +++ etc/services5 May 2021 09:56:49 - @@ -3,10 +3,6 @@ # Network services, Internet style # https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.txt # -# Note that it is presently the policy of IANA to assign a single well-known -# port number for both TCP and UDP; hence, most entries here have two entries -# even if the protocol doesn't support UDP operations. -# tcpmux 1/tcp # TCP port service multiplexer echo 7/tcp @@ -64,10 +60,7 @@ csnet-ns 105/tcp cso-ns # also used by csnet-ns 105/udp cso-ns rtelnet107/tcp # Remote Telnet rtelnet107/udp -pop2 109/tcp postoffice # POP version 2 -pop2 109/udp pop3 110/tcp # POP version 3 -pop3 110/udp sunrpc 111/tcp portmap rpcbind sunrpc 111/udp portmap rpcbind auth 113/tcp authentication tap ident @@ -87,7 +80,6 @@ netbios-dgm 138/udp netbios-ssn139/tcp # NETBIOS session service netbios-ssn139/udp imap 143/tcp imap2 # Internet Message Access Proto -imap 143/udp imap2 # Internet Message Access Proto bftp 152/tcp # Background File Transfer Proto snmp 161/udp # Simple Net Mgmt Proto snmp-trap 162/udp snmptrap# Traps for SNMP @@ -100,11 +92,9 @@ xdmcp 177/udp nextstep 178/tcp NeXTStep NextStep # NeXTStep window nextstep 178/udp NeXTStep NextStep # server bgp179/tcp # Border Gateway Proto. -bgp179/udp prospero 191/tcp # Cliff Neuman's Prospero prospero 191/udp irc194/tcp # Internet Relay Chat -irc194/udp smux 199/tcp # SNMP Unix Multiplexer smux
