Re: xmm(4): WIP diff for Intel XMM7360 LTE modem
Hi, newbie here so please be gentle :-)
Here's a patch adding FCC unlocking as well as setting up IP addresses and
routes. It eliminates the need of that "horrible python thing". Is anyone
still working on the driver or can I go ahead and try and port the ASN1
thing to the kernel?
On Sat, Jul 10, 2021 at 08:20:11PM +, Stuart Henderson wrote:
> On 2021/07/09 17:22, Stuart Henderson wrote:
> > On 2021/07/09 16:33, Stuart Henderson wrote:
> > > Notes so far:
> > >
> > > > +xmmc*)
> > > > + dev=${U%.*}
> > > > + func=${U#*.}
> > > > + M xmmc$U c 101 $(($(($dev*16))+$func)) 660
> > > > + ;;
> > >
> > > "sh MAKEDEV xmmc" isn't enough, it needs "sh MAKEDEV xmmc.0"
> > >
> > > > + ret = EIO;
> > > > + syslog(LOG_ERR, "FCC unlock not implemented, yet");
> > >
> > > Thus ends my initial experimentation ;)
> > >
> >
> > oh; it's just in xmmctl of course! With the horrible python thing, I have
> > a working connection.
>
> If anyone else wants to play, here's a port roughly based on the one in
> pkgsrc. Obviously not optimal but it is nice to be able to see it working.
>
--- xmmctl.c.orig Fri Aug 6 23:52:57 2021
+++ xmmctl.cFri Aug 6 23:54:05 2021
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -247,7 +248,7 @@
asn1_buf_reset(&buf);
ret = read_msg(&buf);
if (ret != 0)
- break;
+ continue;
ret = xmm_msg_decode(&buf, msg);
if (ret != 0) {
xmm_msg_release(msg);
@@ -434,7 +435,10 @@
int
do_fcc_unlock()
{
+ SHA2_CTX ctx;
+ struct asn1_buf buf;
struct xmm_msg msg;
+ uint32_t resp;
int ret = execute(CsiFccLockQueryReq, NULL, true, &msg, "iii");
if (ret != 0) {
@@ -443,17 +447,65 @@
}
/* fcc_mode == 0: No fcc lock required. */
if (msg.val[2].i == 0) {
+ syslog(LOG_DEBUG, "%s: No FCC lock required", __func__);
ret = 0;
goto out;
}
/* fcc_state != 0: already unlocked. */
if (msg.val[1].i != 0) {
+ syslog(LOG_DEBUG, "%s: Already unlocked", __func__);
ret = 0;
goto out;
}
- ret = EIO;
- syslog(LOG_ERR, "FCC unlock not implemented, yet");
+ ret = execute(CsiFccLockGenChallengeReq, NULL, true, &msg, "ii");
+ if (ret != 0) {
+ syslog(LOG_ERR, "CsiFccLockGenChallengeReq failed with %d",
ret);
+ return ret;
+ }
+
+ ret = asn1_buf_init(&buf);
+ if (ret != 0)
+ return ret;
+
+ ret = asn1_add_raw_u32(&buf, 0x19c7f83d);
+ if (ret != 0)
+ return ret;
+
+ ret = asn1_add_raw_u32(&buf, msg.val[1].i);
+ if (ret != 0)
+ return ret;
+
+ SHA256Init(&ctx);
+ SHA256Update(&ctx, buf.buf + buf.off, 8);
+
+ asn1_buf_reset(&buf);
+ ret = asn1_buf_pad(&buf, SHA256_DIGEST_LENGTH);
+ if (ret != 0)
+ return ret;
+
+ SHA256Final(buf.buf + buf.off, &ctx);
+ asn1_decode_raw_u32(&buf, &resp);
+
+ asn1_buf_reset(&buf);
+ ret = asn1_add_int32(&buf, resp);
+ if (ret != 0)
+ return ret;
+
+ ret = execute(CsiFccLockVerChallengeReq, &buf, true, &msg, "i");
+ if (ret != 0) {
+ syslog(LOG_ERR, "CsiFccLockVerChallengeReq failed with %d",
ret);
+ return ret;
+ }
+
+ if (msg.val[0].i != 1) {
+ syslog(LOG_ERR, "FCC unlock failed: %d", ret);
+ return ret;
+ }
+
+ syslog(LOG_DEBUG, "%s: FCC unlocked", __func__);
+
+ ret = 0;
out:
xmm_msg_release(&msg);
return ret;
@@ -764,11 +816,11 @@
*addr = NULL;
- /* XXX assumes that the valid address is followed by 0.0.0.0 */
for (i = 0; i < count; i++) {
- if (addrs[i].s_addr == 0)
- break;
+ if (addrs[i].s_addr != 0) {
*addr = &addrs[i];
+ } else if (*addr)
+ break;
}
if (*addr == NULL)
return EFAULT;
@@ -813,10 +865,11 @@
struct in_addr *ip_addrs = NULL;
const struct in_addr *addr;
struct dns_addr *dns_servers = NULL;
- ssize_t ip_count, dns_count;
+ ssize_t ip_count, dns_count, retp_len;
bool del = false;
char buf[INET6_ADDRSTRLEN];
unsigned int iface_num;
+ FILE *resolv_file;
while ((ch = getopt(argc, argv, "a:p:")) != -1) {
switch (ch) {
@@ -908,25 +961,6 @@
return 1;
}
- /*
-* The value of attach_allowed is unknown at this point.
-* Try to attach even if not allowed (yet). If this fails
-* wait for a status change of attach_allowed (
Re: systat(1) iostat cumulative mode scaling issue
ping
> Hi,
>
> While running systat(1)'s iostat view in cumulative mode (activated by
> pressing
> 'b') I noticed that it divides the results by the refresh interval. This is
> because the cumulative mode is implemented by setting last = 0, and then it
> does
> (current - last) / etime, thereby giving wrong cumulative results. This can be
> verified by comparing with `iostat -dI`. The attached diff fixes this.
>
> ok?
>
> Index: usr.bin/systat/iostat.c
> ===
> RCS file: /cvs/src/usr.bin/systat/iostat.c,v
> retrieving revision 1.49
> diff -u -p -r1.49 iostat.c
> --- usr.bin/systat/iostat.c 28 Jun 2019 13:35:04 - 1.49
> +++ usr.bin/systat/iostat.c 28 Jul 2021 19:04:41 -
> @@ -167,9 +167,12 @@ void
> print_io(void)
> {
> int n, count = 0;
> -
> int curr;
> - etime = naptime;
> +
> + if (state == BOOT)
> + etime = 1.0;
> + else
> + etime = naptime;
>
> /* XXX engine internals: save and restore curr_line for bcache */
> curr = curr_line;
[patch] Preserve keymap adjustments on suspend/resume
According to wsconctl(8), I re-map my Caps Lock key to be a Control:
keyboard.map+='keysym Caps_Lock = Control_L'
After I suspend my machine I lose my modified keyboard layout and it
reverts to the default layout of the chosen encoding.
Looking for a solution I came across a discussion from 2014 where a
similar problem was solved for keyboard encodings [1].
My initial approach was to do just the same for keymaps -- storing
them in parent wsmux on every change. But because of the intricate
interplay between encodings, keymaps and actual hardware keys I had to
look for a simpler solution. Also, it did not feel right to store
keyboard settings in wsmux since it works with abstract event sources.
Here is what I came up with: consolidate configurable keyboard settings
in a data structure which survives attaching/detaching of wskbd.
The behavior described in [1] posed an additional challenge. All
instances of wskbd devices have a shared state which they read and write
-- the default settings for the new keyboards. I am not sure why that
shared encoding (kbd_t) was not protected by a synchronization primitive
previously, but I believe the complex state of encoding+keymap certainly
needs a lock.
I believe in my patch I have preserved the useful behavior described in
[1] regarding interpretation of KB_DEFAULT and carrying last set
encoding as a default encoding for new keyboards. Although, I have no
Sun keyboard to test it.
Resulting behavior:
1. wskbdX devices preserve their individual configs, including custom
keymaps, repeat and bell settings on re-attaching.
2. New keyboards inherit encoding (working since [1]) and custom
keymaps (new) from the last _explicit_ change. Except for keyboards
which have an encoding defined by hardware. Bell and keyrepeat are
excluded from this inheritance because a) it is how it was and b) there
are ioctls to set those defaults explicitly.
Does it look sensible?
Best regards,
Sergii Rudchenko
[1]: https://marc.info/?l=openbsd-misc&m=139051083212925&w=2
diff --git a/sys/dev/wscons/wskbd.c b/sys/dev/wscons/wskbd.c
index ab3bbd3b6f2..12789d72d9d 100644
--- a/sys/dev/wscons/wskbd.c
+++ b/sys/dev/wscons/wskbd.c
@@ -94,6 +94,7 @@
#include
#include
#include
+#include
#include
#include
@@ -122,6 +123,8 @@ int wskbddebug = 0;
#include
struct wskbd_internal {
+ const struct wskbd_mapdata *t_keymap;
+
const struct wskbd_consops *t_consops;
void*t_consaccesscookie;
@@ -136,15 +139,13 @@ struct wskbd_internal {
keysym_t t_symbols[MAXKEYSYMSPERKEY];
struct wskbd_softc *t_sc; /* back pointer */
-
- struct wskbd_mapdata t_keymap; /* translation map table and
- current layout */
};
struct wskbd_softc {
struct wsevsrc sc_base;
struct wskbd_internal *id;
+ struct wskbd_config *sc_config;
const struct wskbd_accessops *sc_accessops;
void *sc_accesscookie;
@@ -164,13 +165,29 @@ struct wskbd_softc {
int sc_translating; /* xlate to chars for emulation */
- int sc_maplen; /* number of entries in sc_map */
- struct wscons_keymap *sc_map; /* current translation map */
-
int sc_refcnt;
u_char sc_dying; /* device is being detached */
};
+/*
+ * Keyboard configuration which can be tuned via wsconsctl and is
+ * persisted across attach/detach cycles (e.g. unplugging and
+ * plugging back a USB keyboard, suspend/resume).
+ */
+struct wskbd_config {
+ struct wskbd_bell_data c_bell_data;
+ struct wskbd_keyrepeat_data c_keyrepeat_data;
+
+ kbd_t c_layout; /* current layout */
+ int c_maplen; /* number of entries in sc_map */
+ struct wscons_keymap *c_map;/* current translation map */
+ const struct wscons_keydesc *c_keydesc; /* map compatibility */
+
+ int c_unit; /* device unit number of the owner */
+ int c_dirty; /* any settings changed since initialization */
+ SLIST_ENTRY(wskbd_config) c_next;
+};
+
#define MOD_SHIFT_L (1 << 0)
#define MOD_SHIFT_R (1 << 1)
#define MOD_SHIFTLOCK (1 << 2)
@@ -203,6 +220,11 @@ intwskbd_match(struct device *, void *, void *);
voidwskbd_attach(struct device *, struct device *, void *);
int wskbd_detach(struct device *, int);
int wskbd_activate(struct device *, int);
+static struct wskbd_config * wskbd_acquire_config(int);
+static voidwskbd_release_config(struct wskbd_config *);
+static voidwskbd_config_copy_defaults(struct wskbd_config *, int);
+static voidwskbd_update_default_keymap(struct wskbd_config *);
+static voidwskbd_config_free_keymap(struct wskbd_config *);
int wskbd_displayioctl(struct device *, u_long, caddr_t, int, struct proc
*);
@@ -257,13 +279,6 @@ extern int kbd_reset;
#define WSKBD_DEFAULT_BELL_VOLUM
Re: date -j and seconds since the Epoch
Hi, sorry for the afterthought, i just noticed that the Subject: line is misleading. This patch has nothing to do with -j. If -j is not specified, this patch changes the value passed to adjtime(2) or settimeofday(2), which arguably matters even more than something merely printed on stdout. However, the value being passed to adjtime(2) or settimeofday(2) right now is just as wrong as the one printed, if -f %s is used, so i think the patch is correct for that case, too. Yours, Ingo
Re: bgpd add add-path receive support
Claudio Jeker(cje...@diehard.n-r-g.com) on 2021.08.04 17:55:45 +0200:
> On Fri, Jul 30, 2021 at 12:02:12PM +0200, Claudio Jeker wrote:
> > This diff implements the bit to support the receive side of
> > RFC7911 - Advertisement of Multiple Paths in BGP.
> >
> > I did some basic tests and it works for me. People running route
> > collectors should give this a try. The interaction of Add-Path and bgpctl
> > probably needs some work. Also the MRT dumper needs to be updated to
> > support RFC8050. I have a partial diff for that ready as well.
> >
> > Sending out multiple paths will follow in a later step since that is a
> > bit more complex. I still need to decide how stable I want to make the
> > assigned path_ids for the multiple paths and then changes to the decision
> > process and adjrib-out are required to allow multipe paths there.
>
> Updated diff that includes some minimal support for bgpctl.
> This add 'bgpctl show rib nei foo path-id 42' as a way to limit which
> paths to show. Now the RFC itself is very flexible in how path-ids are
> assigned (it is possible that different prefixes have different path-ids)
> but the assumtion is that most systems assign path-id in a stable way and
> so it makes sense to allow to filter on path-id.
> Apart from that not much changed.
ok benno@
Only one thing, I worry that using this while the sending side is not working
can lead to
blackholing of prefixes.
>
> --
> :wq Claudio
>
> Index: bgpctl/bgpctl.8
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.8,v
> retrieving revision 1.99
> diff -u -p -r1.99 bgpctl.8
> --- bgpctl/bgpctl.8 16 Jun 2021 16:24:12 - 1.99
> +++ bgpctl/bgpctl.8 4 Aug 2021 13:15:53 -
> @@ -348,6 +348,13 @@ Show RIB memory statistics.
> Show only entries from the specified peer.
> .It Cm neighbor group Ar description
> Show only entries from the specified peer group.
> +.It Cm path-id Ar pathid
> +Show only entries which match the specified
> +.Ar pathid .
> +Must be used together with either
> +.Cm neighbor
> +or
> +.Cm out .
> .It Cm peer-as Ar as
> Show all entries with
> .Ar as
> Index: bgpctl/bgpctl.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
> retrieving revision 1.272
> diff -u -p -r1.272 bgpctl.c
> --- bgpctl/bgpctl.c 2 Aug 2021 16:51:39 - 1.272
> +++ bgpctl/bgpctl.c 4 Aug 2021 15:54:25 -
> @@ -249,6 +249,7 @@ main(int argc, char *argv[])
> ribreq.neighbor = neighbor;
> strlcpy(ribreq.rib, res->rib, sizeof(ribreq.rib));
> ribreq.aid = res->aid;
> + ribreq.path_id = res->pathid;
> ribreq.flags = res->flags;
> imsg_compose(ibuf, type, 0, 0, -1, &ribreq, sizeof(ribreq));
> break;
> Index: bgpctl/parser.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpctl/parser.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 parser.c
> --- bgpctl/parser.c 16 Feb 2021 08:30:21 - 1.106
> +++ bgpctl/parser.c 4 Aug 2021 13:08:31 -
> @@ -61,7 +61,8 @@ enum token_type {
> RD,
> FAMILY,
> RTABLE,
> - FILENAME
> + FILENAME,
> + PATHID,
> };
>
> struct token {
> @@ -114,6 +115,7 @@ static const struct token t_log[];
> static const struct token t_fib_table[];
> static const struct token t_show_fib_table[];
> static const struct token t_communication[];
> +static const struct token t_show_rib_path[];
>
> static const struct token t_main[] = {
> { KEYWORD, "reload", RELOAD, t_communication},
> @@ -178,10 +180,11 @@ static const struct token t_show_rib[] =
> { FLAG, "in", F_CTL_ADJ_IN, t_show_rib},
> { FLAG, "out", F_CTL_ADJ_OUT, t_show_rib},
> { KEYWORD, "neighbor", NONE, t_show_rib_neigh},
> + { KEYWORD, "ovs", NONE, t_show_ovs},
> + { KEYWORD, "path-id", NONE, t_show_rib_path},
> { KEYWORD, "table",NONE, t_show_rib_rib},
> { KEYWORD, "summary", SHOW_SUMMARY, t_show_summary},
> { KEYWORD, "memory", SHOW_RIB_MEM, NULL},
> - { KEYWORD, "ovs", NONE, t_show_ovs},
> { FAMILY, "", NONE, t_show_rib},
> { PREFIX, "", NONE, t_show_prefix},
> { ENDTOKEN, "", NONE, NULL}
> @@ -479,6 +482,11 @@ static const struct token t_show_fib_tab
> { ENDTOKEN, "", NONE, NULL}
> };
>
> +static const struct token t_show_rib_path[] = {
> + { PATHID, "", NONE, t_show_rib},
> + { ENDTOKEN, "", NONE, NULL}
> +};
> +
> static struct parse_result res;
>
> const struct token *mat
Re: date -j and seconds since the Epoch
Hi Gerhard and Bryan, Gerhard Roth wrote on Mon, Aug 02, 2021 at 10:36:05AM +0200: > Bryan Vyhmeister found a strange behavior in date(1): > > # date -f %s -j 1627519989 > Thu Jul 29 01:53:09 PDT 2021 > # date -u -f %s -j 1627519989 > Thu Jul 29 00:53:09 UTC 2021 > > Looks like PDT is GMT-1, which of course is wrong. > > The problem arises from the -f option. The argument of date(1) is passed > to strptime(3). Normally, this will return a broken down time in the > local timezone. This claim confused me somewhat at first. I think a more accurate statement would be that strptime(3) does not use the TZ at all. It merely parses the string and fills the fields in struct tm. The question whether the caller had any particular time zone in mind does not even arise here. Since date(1) says: ENVIRONMENT TZ The time zone to use when parsing or displaying dates. [...] the command $ date -f %H:%M -j 9:00 is correct to essentially just echo "09:00" back at you because date(1) is specified to use the same time zone for parsing and printing. > But the '%s' format makes an exception and returns a > date in UTC. That is indeed true. So for %s the time zone used for parsing is necessarily different, while the time zone used for printing is still the $TZ specified by the user (or the /etc/localtime specified by the sysadmin). So i think your approach of using timegm(3) for %s and mktime(3) otherwise is essentially correct. However, a format string can contain more characters than just a single conversion specification. For example, somebody might wish to parse an input file containing a line SSE=1627519989 and legitimately say $ date -f SSE=%s -j $(grep SSE= input_file) which still yields the wrong result even with your patch. Even worse, what are the admittedly weird commands $ date -f %s:%H -j 1627519989:15 $ date -f %H:%s -j 15:1627519989 $ date -f %H:%s:%m -j 15:1627519989:03 supposed to do? Apparently, data from later conversions is supposed to override data from earlier ones, so should the last conversion that is related to the the time zone win, i.e. %s:%H use mktime(3) but %H:%s and %H:%s:%m gmtime(3)? I would argue that is excessive complexity for little benefit, if any. One might also argue that %s:%H 1627519989:09 is more usefully interpreted as "9 o'clock UTC on the day containing 1627519989" than "9 o'clock in the local TZ on the day containing 1627519989". In addition to using a consistent input time zone, the former also avoids fun with crossing the date line that the latter might cause. The former is also easier to implement, see the patch below. What do people think? > The patch below isn't very beautiful, but fixes the problem: > > # date -f %s -j 1627519989 > Wed Jul 28 17:53:09 PDT 2021 P.S. Your patch was mangled (one missing blank line and spurious spaces inserted before tabs on some lines). Lately, it is becoming annoying that even very experienced developers no longer seem able to reliably send email containing patches that actually apply... :-( Index: date.c === RCS file: /cvs/src/bin/date/date.c,v retrieving revision 1.56 diff -u -p -r1.56 date.c --- date.c 8 Aug 2019 02:17:51 - 1.56 +++ date.c 6 Aug 2021 17:48:01 - @@ -219,7 +219,11 @@ setthetime(char *p, const char *pformat) } /* convert broken-down time to UTC clock time */ - if ((tval = mktime(lt)) == -1) + if (pformat != NULL && strstr(pformat, "%s") != NULL) + tval = timegm(lt); + else + tval = mktime(lt); + if (tval == -1) errx(1, "specified date is outside allowed range"); if (jflag)
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
OK deraadt
Ingo Schwarze wrote:
> Hi,
>
> after quite some head-scratching, i consider the following bug report
> legitimate:
>
> user wrote on Thu, Aug 05, 2021 at 12:43:21AM -0500:
> > On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
> >> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
>
> >>> Less contains a hack to force files of size 0 to become non-seekable
> >>> in order to workaround a linux kernel bug.
>
> I'm inserting a few words into the next sentence to make it clearer
> what it is trying to say:
>
> >>> When the file becomes non-seekable any further reads from the file
> >>> are appended
>
> to the temporary buffer in which less(1) holds the content
> of the file
>
> >>> rather than overwriting the original contents of the file
>
> in that buffer.
>
> >> Bug Reproduction:
>
>$ rm -rf /tmp/test
>
> >> $ touch /tmp/test
> >> $ less /tmp/test
> >> Press r
>
> > $ echo a > /tmp/test# my comment: from a different shell
> > Press h and q in less to reload the file
> > $ echo b > /tmp/test
> > Press h and q in less to reload the file
>
> Now less(1) shows the following on the screen because it thinks
> that would be the current content of the file:
>
> >> a
> >> b
>
> That is wrong. Instead, it should show the actual file content,
> which is just:
>
> b
>
> I think the proposed patch makes sense and should be committed:
> File size has nothing to do with whether a file is seekable,
> so i don't think it can cause regressions.
> Also, it fixes the bug described above.
>
> Any developer willing to provide an OK?
> Alternatively, commit yourself with OK schwarze@.
>
> I'm attaching the patch again because the OP mangled it (tabs
> replaced by spaces) and it did not apply.
>
> Yours,
> Ingo
>
>
> Index: ch.c
> ===
> RCS file: /cvs/src/usr.bin/less/ch.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 ch.c
> --- ch.c 3 Sep 2019 23:08:42 - 1.21
> +++ ch.c 6 Aug 2021 16:14:29 -
> @@ -643,19 +643,6 @@ ch_flush(void)
> ch_block = 0; /* ch_fpos / LBUFSIZE; */
> ch_offset = 0; /* ch_fpos % LBUFSIZE; */
>
> -#if 1
> - /*
> - * This is a kludge to workaround a Linux kernel bug: files in
> - * /proc have a size of 0 according to fstat() but have readable
> - * data. They are sometimes, but not always, seekable.
> - * Force them to be non-seekable here.
> - */
> - if (ch_fsize == 0) {
> - ch_fsize = -1;
> - ch_flags &= ~CH_CANSEEK;
> - }
> -#endif
> -
> if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
> /*
>* Warning only; even if the seek fails for some reason,
>
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
> Any developer willing to provide an OK? ok tb
Re: vmx(4): remove useless code
On Fri, Aug 06, 2021 at 12:06:04PM +0200, Patrick Wildt wrote:
> On Fri, Aug 06, 2021 at 11:05:53AM +0200, Patrick Wildt wrote:
> > Am Thu, Aug 05, 2021 at 02:33:01PM +0200 schrieb Jan Klemkow:
> > > Hi,
> > >
> > > The following diff removes useless code from the driver. As discussed
> > > here [1] and committed there [2], the hypervisor doesn't do anything
> > > with the data structures. We even just set NULL to the pointer since
> > > the initial commit of vmx(4). So, I guess it better to remove all of
> > > these. The variables are bzero'd in vmxnet3_dma_allocmem() anyway.
> > >
> > > OK?
> >
> > My main concern was if the structs are getting zeroed correctly, but
> > they do, so that's fine.
> >
> > That said, it looks like Linux sets the pointer to ~0ULL, not 0. Should
> > we follow Linux' pattern there and do that as well?
> >
>
> Thinking about it a little more, I think we should do that as well. And
> maybe explicitly set driver_data_len to 0 even though it's already zero.
> Basically for readability.
OK?
Index: dev/pci/if_vmx.c
===
RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
retrieving revision 1.66
diff -u -p -r1.66 if_vmx.c
--- dev/pci/if_vmx.c23 Jul 2021 00:29:14 - 1.66
+++ dev/pci/if_vmx.c6 Aug 2021 12:28:51 -
@@ -157,7 +157,6 @@ struct vmxnet3_softc {
#define WRITE_BAR1(sc, reg, val) \
bus_space_write_4((sc)->sc_iot1, (sc)->sc_ioh1, reg, val)
#define WRITE_CMD(sc, cmd) WRITE_BAR1(sc, VMXNET3_BAR1_CMD, cmd)
-#define vtophys(va) 0 /* XXX ok? */
int vmxnet3_match(struct device *, void *, void *);
void vmxnet3_attach(struct device *, struct device *, void *);
@@ -468,8 +467,8 @@ vmxnet3_dma_init(struct vmxnet3_softc *s
ds->vmxnet3_revision = 1;
ds->upt_version = 1;
ds->upt_features = UPT1_F_CSUM | UPT1_F_VLAN;
- ds->driver_data = vtophys(sc);
- ds->driver_data_len = sizeof(struct vmxnet3_softc);
+ ds->driver_data = ~0ULL;
+ ds->driver_data_len = 0;
ds->queue_shared = qs_pa;
ds->queue_shared_len = qs_len;
ds->mtu = VMXNET3_MAX_MTU;
@@ -546,8 +545,8 @@ vmxnet3_alloc_txring(struct vmxnet3_soft
ts->cmd_ring_len = NTXDESC;
ts->comp_ring = comp_pa;
ts->comp_ring_len = NTXCOMPDESC;
- ts->driver_data = vtophys(tq);
- ts->driver_data_len = sizeof *tq;
+ ts->driver_data = ~0ULL;
+ ts->driver_data_len = 0;
ts->intr_idx = intr;
ts->stopped = 1;
ts->error = 0;
@@ -598,8 +597,8 @@ vmxnet3_alloc_rxring(struct vmxnet3_soft
rs->cmd_ring_len[1] = NRXDESC;
rs->comp_ring = comp_pa;
rs->comp_ring_len = NRXCOMPDESC;
- rs->driver_data = vtophys(rq);
- rs->driver_data_len = sizeof *rq;
+ rs->driver_data = ~0ULL;
+ rs->driver_data_len = 0;
rs->intr_idx = intr;
rs->stopped = 1;
rs->error = 0;
Re: less(1): refreshing file of size 0 results in file being treated as a pipe
Hi,
after quite some head-scratching, i consider the following bug report
legitimate:
user wrote on Thu, Aug 05, 2021 at 12:43:21AM -0500:
> On Thu, Aug 05, 2021 at 12:37:00AM -0500, user wrote:
>> On Fri, Jul 23, 2021 at 11:15:59AM -0500, user wrote:
>>> Less contains a hack to force files of size 0 to become non-seekable
>>> in order to workaround a linux kernel bug.
I'm inserting a few words into the next sentence to make it clearer
what it is trying to say:
>>> When the file becomes non-seekable any further reads from the file
>>> are appended
to the temporary buffer in which less(1) holds the content
of the file
>>> rather than overwriting the original contents of the file
in that buffer.
>> Bug Reproduction:
$ rm -rf /tmp/test
>> $ touch /tmp/test
>> $ less /tmp/test
>> Press r
> $ echo a > /tmp/test# my comment: from a different shell
> Press h and q in less to reload the file
> $ echo b > /tmp/test
> Press h and q in less to reload the file
Now less(1) shows the following on the screen because it thinks
that would be the current content of the file:
>> a
>> b
That is wrong. Instead, it should show the actual file content,
which is just:
b
I think the proposed patch makes sense and should be committed:
File size has nothing to do with whether a file is seekable,
so i don't think it can cause regressions.
Also, it fixes the bug described above.
Any developer willing to provide an OK?
Alternatively, commit yourself with OK schwarze@.
I'm attaching the patch again because the OP mangled it (tabs
replaced by spaces) and it did not apply.
Yours,
Ingo
Index: ch.c
===
RCS file: /cvs/src/usr.bin/less/ch.c,v
retrieving revision 1.21
diff -u -p -r1.21 ch.c
--- ch.c3 Sep 2019 23:08:42 - 1.21
+++ ch.c6 Aug 2021 16:14:29 -
@@ -643,19 +643,6 @@ ch_flush(void)
ch_block = 0; /* ch_fpos / LBUFSIZE; */
ch_offset = 0; /* ch_fpos % LBUFSIZE; */
-#if 1
- /*
-* This is a kludge to workaround a Linux kernel bug: files in
-* /proc have a size of 0 according to fstat() but have readable
-* data. They are sometimes, but not always, seekable.
-* Force them to be non-seekable here.
-*/
- if (ch_fsize == 0) {
- ch_fsize = -1;
- ch_flags &= ~CH_CANSEEK;
- }
-#endif
-
if (lseek(ch_file, (off_t)0, SEEK_SET) == (off_t)-1) {
/*
* Warning only; even if the seek fails for some reason,
Re: scp(1) changes in snaps
Damien Miller: > Just a head-up: snaps currently contain a set of changes[1] to > make scp(1) use the SFTP protocol by default. > Please report any incompatibilities or bugs that you encounter here > (tech@), to bugs@ or to openssh@. An obvious cosmetic difference is that relative paths are prefixed with the home directory of the remote source in the progress bar: $ scp lorvorc:foo /dev/null /home/naddy/foo 100% 4099 1.6MB/s 00:00 $ scp -O lorvorc:foo /dev/null foo 100% 4099 3.7MB/s 00:00 I don't know if we care. -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: [External] : TCP missing window update stalls connection
Hello, On Fri, Aug 06, 2021 at 12:26:10AM +0200, Alexander Bluhm wrote: > Hi, > > I have seen some stalling TCP connections while doing unidirectional > throughput tests. The sending machine is doing zero window probes, > but is not sending more data although the other machine announced > that it has space again. > > I guess this commit in tcp_input.c triggered it: > > revision 1.362 > date: 2019/11/11 21:17:21; author: bluhm; state: Exp; lines: +9 -3; > commitid: wXndkYTLO9jvCPdW; > Prevent underflows in tp->snd_wnd if the remote side ACKs more than > tp->snd_wnd. This can happen, for example, when the remote side > responds to a window probe by ACKing the one byte it contains. > from FreeBSD; via markus@; OK sashan@ tobhe@ > > > There we fixed an integer underflow. So we no longer have uint32 > max in the send window, but 0. I see this in my test case. Now > we need the machanism that writes the correct send window size. > > There is a commit in FreeBSD from 2002: > https://urldefense.com/v3/__https://github.com/freebsd/freebsd-src/commit/1645d0903ef7162f5c4516373f0c9df0501ac893__;!!ACWV5N9M2RV99hQ!bS5vG8TH_FAv1Bx-q4iPl5brhCeUQ-GvVeLnfT4b6mk4tY-ktVPZoOAX0zD5ySiQj4R416lx$ > > > The header prediction code did not update snd_wl2. If there is a > sequence number wrap, the send window update is not reached. > > Although I did not obverve it, there seems to be the same problem > for snd_wl1 and rcv_up. For rcv_up I copied the comparison with > rcv_nxt from urgent processing to keep future urgent data. > > ok? > Looks good to me, OK sashan can you also share some details about testing you have done? (tool + command line options) thanks a lot regards sashan
Re: vmx(4): remove useless code
Patrick Wildt wrote: > On Fri, Aug 06, 2021 at 11:05:53AM +0200, Patrick Wildt wrote: > > Am Thu, Aug 05, 2021 at 02:33:01PM +0200 schrieb Jan Klemkow: > > > Hi, > > > > > > The following diff removes useless code from the driver. As discussed > > > here [1] and committed there [2], the hypervisor doesn't do anything > > > with the data structures. We even just set NULL to the pointer since > > > the initial commit of vmx(4). So, I guess it better to remove all of > > > these. The variables are bzero'd in vmxnet3_dma_allocmem() anyway. > > > > > > OK? > > > > My main concern was if the structs are getting zeroed correctly, but > > they do, so that's fine. > > > > That said, it looks like Linux sets the pointer to ~0ULL, not 0. Should > > we follow Linux' pattern there and do that as well? > > > > Thinking about it a little more, I think we should do that as well. And > maybe explicitly set driver_data_len to 0 even though it's already zero. > Basically for readability. The better approach is to zero the whole struct, then fill in non-zero fields... and fill in zero'd fields when doing so idiomaticaly helps future code readers. But if you avoid zero'ing the whole struct, then padding bytes in the struct may be a problem, and depending on the allocator and where the data is later passed to, leak. So full structure zero is the best practice.
Re: Fix unsafe snmpd defaults
Martijn van Duren wrote: > What about something like the phrasing below? It puts a heavy emphasis > on not relying on the defaults (currently the auth and enc keyword > aren't marked as optional in the syntax anyway), but keeps the current > defaults as a strong hint on what is adviced. I also downscaled the > example a little by setting seclevel auth, so it gives a little more > substance to how seclevel can be used. Don't know if that's needed. That is pretty ridiculous. Shall we force users to select their ssh hmacs? Or, maybe you don't mean users, shall we force admins to select their sshd hmacs before sshd will startup? Defaults exist so that systems are easy to use. This discussion has made it clear that there is no security justification, it is simply a case of murdering older hash algorithms EVEN IF THEY ARE SAFELY EMBEDDED INSIDE A HMAC. There are circumstances where the sha256 cult are wrong, and this is one of them.
Re: Fix unsafe snmpd defaults
Theo Buehler wrote: > > hmac-md5 might not be vulnerable, but snmp doesn't use pure hmac-*; in > > the case of md5 and sha1 it strips the result back to 12 bytes (for > > sha256 it's 24 bytes). I'm not saying that is insecure because of it; I > > haven't seen any research on the truncation of HMAC, but when combined > > with known problematic hashing algorithms it doesn't increase my > > confidence level. > > The hash used in an HMAC construction needs much weaker properties than > a cryptographic hash (in particular no collision resistance). Basically, > that's because there's still a private key involved that isn't known to > the attacker. The brokenness of md5 and sha1 isn't strong enough yet to > compromise hmac-md5 and hmac-sha1. > > The rule of thumb is that you need to keep at least half the size of the > hash in a truncated HMAC.Since SHA-1 has 160 bits and MD5 128 bits, > keeping 12 bytes is fine. This corresponds to "96 bits of security". > This isn't all that great by today's standards, but far from completely > bonkers. to conclude, snmpd configuration should be defaulting to the *industry standard* hmac that has highest interopability, because there is no cause for being the first snmp stack to become non-interoperable. The older hmac continues to satisfy the security requirement. The newer mac is causing users problems, because no other systems are configured thus, so you are simply adding an irrational hurdle to snmpd use. The addition of these advanced mac's to the spec is really just ASPIRATIONAL -- they are added for future community use, and possible mainstream adoption in the next decade. There is nothing insecure to fix here. There is no mandate to push people towards maximally non-interoperable defaults. When stated this way, I hope you see it the same way. I recommend changing it back to the industry standard.
Re: Fix unsafe snmpd defaults
> hmac-md5 might not be vulnerable, but snmp doesn't use pure hmac-*; in > the case of md5 and sha1 it strips the result back to 12 bytes (for > sha256 it's 24 bytes). I'm not saying that is insecure because of it; I > haven't seen any research on the truncation of HMAC, but when combined > with known problematic hashing algorithms it doesn't increase my > confidence level. The hash used in an HMAC construction needs much weaker properties than a cryptographic hash (in particular no collision resistance). Basically, that's because there's still a private key involved that isn't known to the attacker. The brokenness of md5 and sha1 isn't strong enough yet to compromise hmac-md5 and hmac-sha1. The rule of thumb is that you need to keep at least half the size of the hash in a truncated HMAC.Since SHA-1 has 160 bits and MD5 128 bits, keeping 12 bytes is fine. This corresponds to "96 bits of security". This isn't all that great by today's standards, but far from completely bonkers.
Re: Fix unsafe snmpd defaults
On Thu, 2021-08-05 at 09:32 +0100, Stuart Henderson wrote: > On 2021/08/03 23:46, Martijn van Duren wrote: > > On Tue, 2021-08-03 at 21:58 +0100, Stuart Henderson wrote: > > > On 2021/08/03 22:07, Martijn van Duren wrote: > > > > On Tue, 2021-08-03 at 18:24 +0100, Stuart Henderson wrote: > > > > > On 2021/06/15 17:39, Stuart Henderson wrote: > > > > > > > Then again, I don't get the feeling many people use snmpd at this > > > > > > > time > > > > > > > and maybe it's a good moment to bite the bullet and go for safest > > > > > > > defaults possible at this time. But if that's the case I would > > > > > > > like to > > > > > > > follow up with a diff to changes the default auth to hmac-sha512, > > > > > > > because snmp drops trailing bytes of the result and enc to aes > > > > > > > instead > > > > > > > of des. > > > > > > > > > > > > This is the change that feels most likely to affect existing SNMPv3 > > > > > > users. > > > > > > Support in management software beyond aes/sha1 is a bit lacking and > > > > > > prone > > > > > > to incompatibility (I had issues with net-snmp and snmpd using > > > > > > hmac-sha256 > > > > > > though it seems it will work with hmac-sha512..) > > > > > > > > > > BTW, having updated a few machines now, I am finding the change to > > > > > sha2-256 by default to be a complete pain, especially considering that > > > > > /etc/examples/snmpd.conf uses "enc aes" but has no setting for auth > > > > > so relies on defaults for that.. > > > > > > > > > I can't do a lot with "a complete pain". > > > > > > > > Does something like the diff below make things more intuitive? If not, > > > > could you be a little more concrete? > > > > > > > > martijn@ > > > > > > > > Index: snmpd.conf > > > > === > > > > RCS file: /cvs/src/etc/examples/snmpd.conf,v > > > > retrieving revision 1.1 > > > > diff -u -p -r1.1 snmpd.conf > > > > --- snmpd.conf 11 Jul 2014 21:20:10 - 1.1 > > > > +++ snmpd.conf 3 Aug 2021 20:05:53 - > > > > @@ -18,7 +18,9 @@ system services 74 > > > > oid 1.3.6.1.4.1.30155.42.3.1 name testStringValue read-only string > > > > "Test" > > > > oid 1.3.6.1.4.1.30155.42.3.4 name testIntValue read-write integer 1 > > > > > > > > -# Enable SNMPv3 USM with authentication, encryption and two defined > > > > users > > > > -#seclevel enc > > > > -#user "user1" authkey "password123" enc aes enckey "321drowssap" > > > > -#user "user2" authkey "password456" enckey "654drowssap" > > > > +# Create two SNMPv3 USM users: > > > > +# User with default crypto values > > > > +#user "defaultuser" authkey "password123" enckey "321drowssap" > > > > +# User with backwards compatible crypto: > > > > +# Only enable and use when client absolutely can't deal with modern > > > > defaults. > > > > +#user "compatuser" authkey "password456" auth hmac-md5 enckey > > > > "654drowssap" enc des > > > > > > > > > > > > > > Given the lack of support for SHA2-256 in much management software until > > > recently AES+SHA is a pretty common configuration. And given the old > > > snmpd.conf > > > example I think that is often done by copying/editing so just "enc aes" > > > is there > > > with no auth setting. Wondering if that part might not have been such a > > > good > > > change and what anyone else thinks.. > > > > > I think that these management software applications should join 2016 and > > start > > implementing it and until then its just two or four minor keywords per user. > > But I'm not a heavy user of 3rd party mangement software. > > "Given the lack of support for SHA2-256 in much management software > _until recently_". What they do now is only relevant to new > configurations, it's what they did in the past when they were configured > that determines what people are using in existing config and whether > they'll have to reconfigure things to cope with the default changing. Fair enough, I overlooked that part. However is it sane to rely on the defaults of application for a protocol that doesn't dynamically adapt to changes to the underlying crypto? > > Since even hmac-md5 is still AFAIK not considered unsafe (hmac is a > very different thing to using the algorithm for file integrity checks) > and there are clearly still issues with sha2-256 in SNMP I think we're > probably better off reverting that part of the defaults change and > go back to hmac-sha1. hmac-md5 might not be vulnerable, but snmp doesn't use pure hmac-*; in the case of md5 and sha1 it strips the result back to 12 bytes (for sha256 it's 24 bytes). I'm not saying that is insecure because of it; I haven't seen any research on the truncation of HMAC, but when combined with known problematic hashing algorithms it doesn't increase my confidence level. > > Diff for that below - it also fixes some text missed in the previous > change des->aes, A yes, missed that one. Thanks for spotting that. Regardless of anything else: that part is OK marti
Re: vmx(4): remove useless code
On Fri, Aug 06, 2021 at 11:05:53AM +0200, Patrick Wildt wrote:
> Am Thu, Aug 05, 2021 at 02:33:01PM +0200 schrieb Jan Klemkow:
> > Hi,
> >
> > The following diff removes useless code from the driver. As discussed
> > here [1] and committed there [2], the hypervisor doesn't do anything
> > with the data structures. We even just set NULL to the pointer since
> > the initial commit of vmx(4). So, I guess it better to remove all of
> > these. The variables are bzero'd in vmxnet3_dma_allocmem() anyway.
> >
> > OK?
>
> My main concern was if the structs are getting zeroed correctly, but
> they do, so that's fine.
>
> That said, it looks like Linux sets the pointer to ~0ULL, not 0. Should
> we follow Linux' pattern there and do that as well?
>
Thinking about it a little more, I think we should do that as well. And
maybe explicitly set driver_data_len to 0 even though it's already zero.
Basically for readability.
>
> > bye,
> > Jan
> >
> > [1]: https://www.lkml.org/lkml/2021/1/19/1225
> > [2]:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/vmxnet3/vmxnet3_drv.c?id=de1da8bcf40564a2adada2d5d5426e05355f66e8
> >
> > Index: dev/pci/if_vmx.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
> > retrieving revision 1.66
> > diff -u -p -r1.66 if_vmx.c
> > --- dev/pci/if_vmx.c23 Jul 2021 00:29:14 - 1.66
> > +++ dev/pci/if_vmx.c5 Aug 2021 11:12:26 -
> > @@ -157,7 +157,6 @@ struct vmxnet3_softc {
> > #define WRITE_BAR1(sc, reg, val) \
> > bus_space_write_4((sc)->sc_iot1, (sc)->sc_ioh1, reg, val)
> > #define WRITE_CMD(sc, cmd) WRITE_BAR1(sc, VMXNET3_BAR1_CMD, cmd)
> > -#define vtophys(va) 0 /* XXX ok? */
> >
> > int vmxnet3_match(struct device *, void *, void *);
> > void vmxnet3_attach(struct device *, struct device *, void *);
> > @@ -468,8 +467,6 @@ vmxnet3_dma_init(struct vmxnet3_softc *s
> > ds->vmxnet3_revision = 1;
> > ds->upt_version = 1;
> > ds->upt_features = UPT1_F_CSUM | UPT1_F_VLAN;
> > - ds->driver_data = vtophys(sc);
> > - ds->driver_data_len = sizeof(struct vmxnet3_softc);
> > ds->queue_shared = qs_pa;
> > ds->queue_shared_len = qs_len;
> > ds->mtu = VMXNET3_MAX_MTU;
> > @@ -546,8 +543,6 @@ vmxnet3_alloc_txring(struct vmxnet3_soft
> > ts->cmd_ring_len = NTXDESC;
> > ts->comp_ring = comp_pa;
> > ts->comp_ring_len = NTXCOMPDESC;
> > - ts->driver_data = vtophys(tq);
> > - ts->driver_data_len = sizeof *tq;
> > ts->intr_idx = intr;
> > ts->stopped = 1;
> > ts->error = 0;
> > @@ -598,8 +593,6 @@ vmxnet3_alloc_rxring(struct vmxnet3_soft
> > rs->cmd_ring_len[1] = NRXDESC;
> > rs->comp_ring = comp_pa;
> > rs->comp_ring_len = NRXCOMPDESC;
> > - rs->driver_data = vtophys(rq);
> > - rs->driver_data_len = sizeof *rq;
> > rs->intr_idx = intr;
> > rs->stopped = 1;
> > rs->error = 0;
> >
>
Re: vmx(4): remove useless code
Am Thu, Aug 05, 2021 at 02:33:01PM +0200 schrieb Jan Klemkow:
> Hi,
>
> The following diff removes useless code from the driver. As discussed
> here [1] and committed there [2], the hypervisor doesn't do anything
> with the data structures. We even just set NULL to the pointer since
> the initial commit of vmx(4). So, I guess it better to remove all of
> these. The variables are bzero'd in vmxnet3_dma_allocmem() anyway.
>
> OK?
My main concern was if the structs are getting zeroed correctly, but
they do, so that's fine.
That said, it looks like Linux sets the pointer to ~0ULL, not 0. Should
we follow Linux' pattern there and do that as well?
Patrick
> bye,
> Jan
>
> [1]: https://www.lkml.org/lkml/2021/1/19/1225
> [2]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/vmxnet3/vmxnet3_drv.c?id=de1da8bcf40564a2adada2d5d5426e05355f66e8
>
> Index: dev/pci/if_vmx.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_vmx.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 if_vmx.c
> --- dev/pci/if_vmx.c 23 Jul 2021 00:29:14 - 1.66
> +++ dev/pci/if_vmx.c 5 Aug 2021 11:12:26 -
> @@ -157,7 +157,6 @@ struct vmxnet3_softc {
> #define WRITE_BAR1(sc, reg, val) \
> bus_space_write_4((sc)->sc_iot1, (sc)->sc_ioh1, reg, val)
> #define WRITE_CMD(sc, cmd) WRITE_BAR1(sc, VMXNET3_BAR1_CMD, cmd)
> -#define vtophys(va) 0/* XXX ok? */
>
> int vmxnet3_match(struct device *, void *, void *);
> void vmxnet3_attach(struct device *, struct device *, void *);
> @@ -468,8 +467,6 @@ vmxnet3_dma_init(struct vmxnet3_softc *s
> ds->vmxnet3_revision = 1;
> ds->upt_version = 1;
> ds->upt_features = UPT1_F_CSUM | UPT1_F_VLAN;
> - ds->driver_data = vtophys(sc);
> - ds->driver_data_len = sizeof(struct vmxnet3_softc);
> ds->queue_shared = qs_pa;
> ds->queue_shared_len = qs_len;
> ds->mtu = VMXNET3_MAX_MTU;
> @@ -546,8 +543,6 @@ vmxnet3_alloc_txring(struct vmxnet3_soft
> ts->cmd_ring_len = NTXDESC;
> ts->comp_ring = comp_pa;
> ts->comp_ring_len = NTXCOMPDESC;
> - ts->driver_data = vtophys(tq);
> - ts->driver_data_len = sizeof *tq;
> ts->intr_idx = intr;
> ts->stopped = 1;
> ts->error = 0;
> @@ -598,8 +593,6 @@ vmxnet3_alloc_rxring(struct vmxnet3_soft
> rs->cmd_ring_len[1] = NRXDESC;
> rs->comp_ring = comp_pa;
> rs->comp_ring_len = NRXCOMPDESC;
> - rs->driver_data = vtophys(rq);
> - rs->driver_data_len = sizeof *rq;
> rs->intr_idx = intr;
> rs->stopped = 1;
> rs->error = 0;
>
