Re: Division by zero in ehci(4)

2016-09-18 Thread Jonathan Gray
On Sat, Sep 17, 2016 at 04:36:12PM +0200, Martin Pieuchot wrote:
> One of the non-checked value read from an untrusted descriptor is the
> "maximum packet size" of an endpoint.  If a device reports an incorrect
> value most of our HC drivers wont work and if this value is 0 ehci(4)
> will crash the kernel.
> 
> So here's a diff to validate the value read from the device descriptor
> which ends up being the value of the default endpoint.
> 
> ok?

This patch made vmware hang when attaching xhci uhubs.

usbd_new_device bus=0x801e4000 port=0 depth=0 speed=4
usbd_new_device: adding unit addr=1, rev=300, class=9, subclass=0, protocol=1, 
maxpacket=9, len=18, speed=4
usb2: root hub problem

usbd_new_device mps 9 mps0 512

It would appear that for superspeed devices maxpacketsize0 is a power
of 2?  ie 2^9 is 512.

Bus 000 Device 001: ID 15ad: VMware Inc. 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   2.00
  bDeviceClass9 Hub
  bDeviceSubClass 0 Unused
  bDeviceProtocol 1 Single TT
  bMaxPacketSize064
  idVendor   0x15ad VMware Inc.
  idProduct  0x 
  bcdDevice1.00
  iManufacturer   1 VMware
  iProduct2 EHCI root hub
  iSerial 0 
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   25
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0x40
  (Missing must-be-set bit!)
  Self Powered
MaxPower0mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0 Unused
  bInterfaceProtocol  0 Full speed (or root) hub
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0008  1x 8 bytes
bInterval  12
Hub Descriptor:
  bLength  10
  bDescriptorType  41
  nNbrPorts 6
  wHubCharacteristic 0x0002
No power switching (usb 1.0)
Ganged overcurrent protection
TT think time 8 FS bits
  bPwrOn2PwrGood  200 * 2 milli seconds
  bHubContrCurrent  0 milli Ampere
  DeviceRemovable0x00
  PortPwrCtrlMask0x00
 Hub Port Status:
   Port 1: .0500 highspeed power
   Port 2: .0500 highspeed power
   Port 3: .0500 highspeed power
   Port 4: .0500 highspeed power
   Port 5: .0500 highspeed power
   Port 6: .0500 highspeed power
Device Status: 0x0001
  Self Powered

Bus 001 Device 001: ID 15ad: VMware Inc. 
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   1.00
  bDeviceClass9 Hub
  bDeviceSubClass 0 Unused
  bDeviceProtocol 0 Full speed (or root) hub
  bMaxPacketSize064
  idVendor   0x15ad VMware Inc.
  idProduct  0x 
  bcdDevice1.00
  iManufacturer   1 VMware
  iProduct2 UHCI root hub
  iSerial 0 
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   25
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0x40
  (Missing must-be-set bit!)
  Self Powered
MaxPower0mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   1
  bInterfaceClass 9 Hub
  bInterfaceSubClass  0 Unused
  bInterfaceProtocol  0 Full speed (or root) hub
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes3
  Transfer TypeInterrupt
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0008  1x 8 bytes
bInterval 255
Device Status: 0x0001
  Self Powered

Bus 001 Device 002: ID 0e0f:0002 VMware, Inc. Virtual USB Hub
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   1.10
  bDeviceClass9 Hub
  bDeviceSubClass 0 Unused
  bDeviceProtocol 0 Full speed (or root) hub
  bMaxPacketSize0 8
  idVendor   0x0e0f VMware, 

Re: timeout_set_proc(9)

2016-09-18 Thread David Gwynne
On Fri, Sep 16, 2016 at 04:58:39PM +0200, Mark Kettenis wrote:
> > Date: Thu, 15 Sep 2016 16:29:45 +0200
> > From: Martin Pieuchot 
> > 
> > After discussing with a few people about a new "timed task" API I came
> > to the conclusion that mixing timeouts and tasks will result in:
> > 
> >   - always including a 'struct timeout' in a 'struct task', or the other
> > the way around
> > or
> >   
> >   - introducing a new data structure, hence API.
> > 
> > Since I'd like to keep the change as small as possible when converting
> > existing timeout_set(9), neither option seem a good fit.  So I decided
> > to add a new kernel thread, curiously named "softclock", that will
> > offer his stack to the poor timeout handlers that need one. 
> > 
> > With this approach, converting a timeout is just a matter of doing:
> > 
> > s/timeout_set/timeout_set_proc/
> > 
> > 
> > Diff below includes the conversions I need for the "netlock".  I'm
> > waiting for feedbacks and a better name to document the new function.
> > 
> > Comments?
> 
> I like how minimal this is.  Would like to see a few more people that
> are familliar with the timeout code chime in, but it looks mostly
> correct to me as well.  One question though:

id rather not grow the timeout (or task for that matter) apis just
yet. theyre nice and straightforward to read and understand so far.

for this specific problem can we do a specific fix for it? the diff
below is equiv to the timeout_set_proc change, but implements it
by using explicit tasks that are called by the timeouts from a
common trampoline in the network stack.

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.448
diff -u -p -r1.448 if.c
--- net/if.c13 Sep 2016 08:15:01 -  1.448
+++ net/if.c19 Sep 2016 01:51:37 -
@@ -155,6 +155,8 @@ voidif_watchdog_task(void *);
 void   if_input_process(void *);
 void   if_netisr(void *);
 
+void   if_nettmo_add(void *);
+
 #ifdef DDB
 void   ifa_print_all(void);
 #endif
@@ -875,6 +877,21 @@ if_netisr(void *unused)
 
splx(s);
KERNEL_UNLOCK();
+}
+
+void
+if_nettmo_add(void *t)
+{
+   /* the task is added to systq so it inherits the KERNEL_LOCK */
+   task_add(systq, t);
+}
+
+void
+if_nettmo_set(struct timeout *tmo, struct task *task,
+void (*fn)(void *), void *arg)
+{
+   task_set(task, fn, arg);
+   timeout_set(tmo, if_nettmo_add, task);
 }
 
 void
Index: net/if_pflow.c
===
RCS file: /cvs/src/sys/net/if_pflow.c,v
retrieving revision 1.61
diff -u -p -r1.61 if_pflow.c
--- net/if_pflow.c  29 Apr 2016 08:55:03 -  1.61
+++ net/if_pflow.c  19 Sep 2016 01:51:37 -
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -547,16 +548,24 @@ pflow_init_timeouts(struct pflow_softc *
timeout_del(>sc_tmo6);
if (timeout_initialized(>sc_tmo_tmpl))
timeout_del(>sc_tmo_tmpl);
-   if (!timeout_initialized(>sc_tmo))
-   timeout_set(>sc_tmo, pflow_timeout, sc);
+   if (!timeout_initialized(>sc_tmo)) {
+   if_nettmo_set(>sc_tmo, >sc_task,
+   pflow_timeout, sc);
+   }
break;
case PFLOW_PROTO_10:
-   if (!timeout_initialized(>sc_tmo_tmpl))
-   timeout_set(>sc_tmo_tmpl, pflow_timeout_tmpl, sc);
-   if (!timeout_initialized(>sc_tmo))
-   timeout_set(>sc_tmo, pflow_timeout, sc);
-   if (!timeout_initialized(>sc_tmo6))
-   timeout_set(>sc_tmo6, pflow_timeout6, sc);
+   if (!timeout_initialized(>sc_tmo_tmpl)) {
+   if_nettmo_set(>sc_tmo_tmpl, >sc_task_tmpl,
+   pflow_timeout_tmpl, sc);
+   }
+   if (!timeout_initialized(>sc_tmo)) {
+   if_nettmo_set(>sc_tmo, >sc_task,
+   pflow_timeout, sc);
+   }
+   if (!timeout_initialized(>sc_tmo6)) {
+   if_nettmo_set(>sc_tmo6, >sc_task6,
+   pflow_timeout6, sc);
+   }
 
timeout_add_sec(>sc_tmo_tmpl, PFLOW_TMPL_TIMEOUT);
break;
Index: net/if_pflow.h
===
RCS file: /cvs/src/sys/net/if_pflow.h,v
retrieving revision 1.14
diff -u -p -r1.14 if_pflow.h
--- net/if_pflow.h  3 Oct 2015 10:44:23 -   1.14
+++ net/if_pflow.h  19 Sep 2016 01:51:37 -
@@ -182,8 +182,11 @@ struct pflow_softc {
u_int64_tsc_gcounter;
u_int32_tsc_sequence;
struct timeout   sc_tmo;
-   struct timeout   sc_tmo6;
+   

Re: LibreSSL selects weak digest for (EC)DH

2016-09-18 Thread kinichiro inoguchi
Thanks, Brent.
I appreciate if you commit this.

Kinichiro


Re: LibreSSL selects weak digest for (EC)DH

2016-09-18 Thread Brent Cook
Looks fine to me. Shall I commit it?

On Thu, Sep 15, 2016 at 2:32 AM, Kinichiro Inoguchi <
kinichiro.inogu...@gmail.com> wrote:

> Sorry, I attached wrong patch file.
> I re-post the patch file again.
>
> On Thu, Sep 15, 2016 at 04:10:55PM +0900, Kinichiro Inoguchi wrote:
> > Hi,
> >
> > I would like to fix this SNI issue.
> >
> > reported by @davidben
> > https://github.com/libressl-portable/openbsd/issues/69
> >
> > #3560: OpenSSL selects weak digest for (EC)DH
> > https://rt.openssl.org/Ticket/Display.html?id=3560
> >
> > original OpenSSL commit is here.
> > https://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=
> 4e05aedbcab7f7f83a887e952ebdcc5d4f2291e4
> >
> > I will add a patch for this.
> > ok ?
>


Re: alignment error rtadvd/armv7

2016-09-18 Thread Martin Brandenburg
On Sun, 18 Sep 2016, Jeremie Courreges-Anglas wrote:

> Martin Brandenburg  writes:
> 
> > On a PandaBoard (armv7) running -current, when I run rtadvd, it crashes
> > with a bus error shortly after printing (received a routing message). I
> > can reproduce by sending SIGHUP to a dhclient running on the same
> > interface.
> >
> > I have traced this down to the following block of code in rtadvd.c.
> >
> > static void
> > rtmsg_input(void)
> > {
> > int n, type, ifindex = 0, plen;
> > size_t len;
> > char msg[2048], *next, *lim;
> > u_char ifname[IF_NAMESIZE];
> > struct prefix *prefix;
> > struct rainfo *rai;
> > struct in6_addr *addr;
> > char addrbuf[INET6_ADDRSTRLEN];
> >
> > So msg is not 32-bit aligned, presumably because INET6_ADDRSTRLEN is 46.
> > I can fix the bus error by hardcoding 48, but of course that's not
> > right.
> >
> > Then msg is passed to get_next_msg (as next) where the expression
> > rtm->rtm_hdrlen (rtm is the not-aligned msg) is the first dereference
> > and thus the point where it crashes.
> >
> > I'm at the point now where I think I've found the root of the problem
> > but don't know enough to fix it.
> >
> > Any thoughts?
> 
> Thanks for the report.
> 
> I guess that we could fix the rtm_* functions to work on an unaligned
> input buffer, but an easier fix would be to just ask for a suitably
> aligned input buffer, with malloc(3).  Does the diff below fix your
> problem?

This fixes the problem. I let it sit in debug mode for 30 minutes (which
is far longer than it ever lasted before) through plenty of routing
messages, and it never crashed. I will keep monitoring, but I think it's
good.

Martin



Re: [PATCH] www: mention /usr/src in upgrade notes

2016-09-18 Thread Theo Buehler
On Mon, Sep 19, 2016 at 01:59:20AM +0200, Simon Ruderich wrote:
> I'm a relatively new OpenBSD user and just updated my system from
> 5.9 to 6.0 but forgot to update the files in /usr/src. Thus when
> I applied the latest patches (001-006) I actually built an old
> 5.9 kernel which failed to boot. The following patch mentions
> /usr/src in the upgrade notes.

Thanks a lot for taking the time and submitting a patch. I will need to
think about this a little. I will probably end up adding a link to
errata.html since that is a more glaring omission on that page.

I think the real problem is that our documentation of -stable is
suboptimal in that there is this tangle of very similar, but
not-quite-the-same, information on stable.html, anoncvs.html and
faq5.html and none of these pages really cuts to the chase.

Improving this is on my to-do list for 6.1, but I haven't gotten around
to figuring out a good solution for that.  Moreover, I'm holding off
because there is ongoing work that might affect this:
http://undeadly.org/cgi?action=article=20160911012316



Re: little simpler ssh code

2016-09-18 Thread Darren Tucker
On Mon, Sep 19, 2016 at 5:41 AM, Martin Natano  wrote:

> Two more loops that can be converted to arc4random_buf(). Ok?
> [...]
> +   arc4random_buf(x11_fake_data, data_len);
> for (i = 0; i < data_len; i++) {
>

I'd put that below the for loop so it matches the order of the comment
above ("Extract real authentication data and generate fake data of the same
length.").  Or better yet, change the comment and group all of the
operations on x11_fake_data and x11_fake_data_len together.

[...]

> +   arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
>

sizeof(session_key) instead please.

with those, ok dtucker@

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860  37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.


[PATCH] www: mention /usr/src in upgrade notes

2016-09-18 Thread Simon Ruderich
Hello,

I'm a relatively new OpenBSD user and just updated my system from
5.9 to 6.0 but forgot to update the files in /usr/src. Thus when
I applied the latest patches (001-006) I actually built an old
5.9 kernel which failed to boot. The following patch mentions
/usr/src in the upgrade notes.

Regards
Simon

Index: upgrade60.html
===
RCS file: /cvs/www/faq/upgrade60.html,v
retrieving revision 1.11
diff -u -r1.11 upgrade60.html
--- upgrade60.html  14 Sep 2016 15:53:09 -  1.11
+++ upgrade60.html  18 Sep 2016 23:55:55 -
@@ -41,7 +41,8 @@
 boot loader to boot this kernel.
 Once this kernel is booted, choose the (U)pgrade option and follow the
 prompts.
-Finish up by upgrading the packages: pkg_add -u.
+Finish up by upgrading the packages: pkg_add -u. Don't forget
+to update /usr/src if you're using it to build from source.
 
 
 Alternatively, you can use the manual upgrade 
process.

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9


signature.asc
Description: PGP signature


Re: video(1): Don't limit maximum frame size to display size

2016-09-18 Thread Jeremie Courreges-Anglas
Gregor Best  writes:

> Hi,
>
> this is another patch to video(1). It removes the logic that forces the
> video grab frame size to be <= the size of the Xv adaptor. This allows
> me to grab and view frames that are 1280x1024 on my 1440x900 screen. Xv
> properly scales and stretches the window content, so there's nothing
> that gets cut off from the video feed.
>
> The camera I have here has the following resolutions:
>
> 160x120: 30
> 176x144: 30
> 320x240: 30
> 352x288: 30
> 640x480: 30
> 1280x1024: 9
>
> Without this patch, the maximum resolution I can grab and view at the
> same time is 640x480, which is a bit too tiny.

This seems to work fine with inteldrm, a 1280x800 display and

video device /dev/video:
  encodings: yuy2
  frame sizes (width x height, in pixels) and rates (in frames per second):
320x240: 30
352x288: 30
640x480: 30
800x600: 15
1024x768: 12
1280x720: 7
1280x800: 7
1600x1200: 5
  controls: brightness, contrast, saturation, hue, gamma, sharpness

> I haven't noticed any
> downsides of the patch, but I've only tested it with my laptop and not
> other graphics cards, so I'm not sure how Xv handles frames larger than
> the display size there.

No idea either... if you wanted to reduce the risk of breakage, I guess
you could limit the resolution only if -O is not passed.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: Manpage for uctuctl(4)

2016-09-18 Thread Ingo Schwarze
Hi Rafael,

Rafael Neves wrote on Sun, Sep 18, 2016 at 04:48:37PM +0100:

> I have to admit that the idea of derived work in such short
> content manpage is very ethereal for me. :)

I think you are not alone.  If i understand correctly, disputes
over Copyright of small amounts of content end up in court more
easily than disputes about large amounts of content, both regarding
works that are very small in the first place (because users tend
to argue they are so trivial they don't deserve Copyright protection
at all) and regarding very small extracts from larger works (because
brevity of an extract is one of the criteria that helps to argue
for fair use).

Anyway, i have committed an initial version of the manual.
In case it still needs tweaking, that can be done in the tree.

Yours,
  Ingo



video(1): use _NET_WM_STATE_FULLSCREEN

2016-09-18 Thread Dmitrij D. Czarkoff
Hi!

The diff below fixes fullscreen mode on window managers that prevent
applications from resizing their windows.  This is done by setting
_NET_WM_STATE_FULLSCREEN atom.

--
Dmitrij D. Czarkoff

Index: video.c
===
RCS file: /var/cvs/xenocara/app/video/video.c,v
retrieving revision 1.19
diff -u -p -r1.19 video.c
--- video.c 6 Jun 2016 19:31:22 -   1.19
+++ video.c 18 Sep 2016 21:33:29 -
@@ -160,6 +160,7 @@ struct video {
int  conv_type;
int  enc;
int  full_screen;
+   int  net_wm;
int  width;
int  height;
int  bpf;
@@ -178,6 +179,7 @@ int xv_get_info(struct video *);
 int xv_sel_adap(struct video *);
 void xv_dump_info(struct video *);
 int xv_init(struct video *);
+void net_wm_supported(struct video *);
 void resize_window(struct video *, int);
 void display_event(struct video *);
 
@@ -500,34 +502,84 @@ xv_init(struct video *vid)
 }
 
 void
+net_wm_supported(struct video *vid)
+{
+   struct xdsp *x = >xdsp;
+   Atom query, fullscreen;
+   Atom type;
+   Atom *data;
+   long off;
+   int fmt, len = 12;
+   unsigned long nitems, remain;
+   int i;
+
+   query = XInternAtom(x->dpy, "_NET_SUPPORTED", True);
+   fullscreen = XInternAtom(x->dpy, "_NET_WM_STATE_FULLSCREEN", True);
+
+   for (off = 0; XGetWindowProperty(x->dpy, x->rwin, query, off,
+ len, False, XA_ATOM, , , , ,
+ (unsigned char **)) == Success; off += len) {
+   if (type == XA_ATOM && fmt == 32) {
+   for (i = 0; i < nitems; i++) {
+   if (data[i] == fullscreen) {
+   vid->net_wm = 1;
+   XFree(data);
+   return;
+   }
+   }
+   }
+   XFree(data);
+   }
+
+   return;
+}
+
+void
 resize_window(struct video *vid, int fullscreen)
 {
struct xdsp *x = >xdsp;
XWindowAttributes winatt;
Window junk;
-   int new_width;
-   int new_height;
int new_x;
int new_y;
+   XEvent ev;
+   Atom property, value;
 
if (fullscreen == 1) {
if (vid->full_screen == 1) {
-   new_width = x->saved_w;
-   new_height = x->saved_h;
+   x->width = x->saved_w;
+   x->height = x->saved_h;
new_x = x->saved_x;
new_y = x->saved_y;
-   vid->full_screen = 0;
} else {
-   new_width = x->max_width;
-   new_height = x->max_height;
+   x->width = x->max_width;
+   x->height = x->max_height;
new_x = 0;
new_y = 0;
-   vid->full_screen = 1;
}
-   x->width = new_width;
-   x->height = new_height;
-   XMoveResizeWindow(x->dpy, x->window, new_x, new_y,
-   new_width, new_height);
+
+   vid->full_screen = !vid->full_screen;
+
+   if (vid->net_wm) {
+   property = XInternAtom(x->dpy, "_NET_WM_STATE", False);
+   value = XInternAtom(x->dpy, "_NET_WM_STATE_FULLSCREEN",
+   False);
+
+   memset(, 0, sizeof(ev));
+   ev.type = ClientMessage;
+   ev.xclient.window = x->window;
+   ev.xclient.message_type = property;
+   ev.xclient.format = 32;
+   ev.xclient.data.l[0] = vid->full_screen;
+   ev.xclient.data.l[1] = value;
+
+   XSendEvent(x->dpy, x->rwin, False,
+   SubstructureNotifyMask | SubstructureRedirectMask,
+   );
+   } else {
+   XMoveResizeWindow(x->dpy, x->window, x->width,
+   x->height, new_x, new_y);
+   }
} else if (!vid->full_screen) {
XGetWindowAttributes(x->dpy, x->window, );
XTranslateCoordinates(x->dpy, x->window, x->rwin,
@@ -536,8 +588,8 @@ resize_window(struct video *vid, int ful
x->saved_h = x->height = winatt.height;
x->saved_x = new_x;
x->saved_y = new_y;
-   XResizeWindow(x->dpy, x->window, x->width, x->height);
}
+
XSync(x->dpy, False);
XSync(x->dpy, True);
 }
@@ -1435,6 +1487,9 @@ setup(struct video *vid)
if (!mmap_init(vid))
return 0;
}
+
+

Re: little simpler ssh code

2016-09-18 Thread Martin Natano
Two more loops that can be converted to arc4random_buf(). Ok?

natano


Index: channels.c
===
RCS file: /cvs/src/usr.bin/ssh/channels.c,v
retrieving revision 1.352
diff -u -p -r1.352 channels.c
--- channels.c  12 Sep 2016 01:22:38 -  1.352
+++ channels.c  18 Sep 2016 19:04:30 -
@@ -4148,7 +4148,6 @@ x11_request_forwarding_with_spoofing(int
char *new_data;
int screen_number;
const char *cp;
-   u_int32_t rnd = 0;
 
if (x11_saved_display == NULL)
x11_saved_display = xstrdup(disp);
@@ -4175,15 +4174,12 @@ x11_request_forwarding_with_spoofing(int
 */
x11_saved_data = xmalloc(data_len);
x11_fake_data = xmalloc(data_len);
+   arc4random_buf(x11_fake_data, data_len);
for (i = 0; i < data_len; i++) {
if (sscanf(data + 2 * i, "%2x", ) != 1)
fatal("x11_request_forwarding: bad "
"authentication data: %.100s", data);
-   if (i % 4 == 0)
-   rnd = arc4random();
x11_saved_data[i] = value;
-   x11_fake_data[i] = rnd & 0xff;
-   rnd >>= 8;
}
x11_saved_data_len = data_len;
x11_fake_data_len = data_len;
Index: sshconnect1.c
===
RCS file: /cvs/src/usr.bin/ssh/sshconnect1.c,v
retrieving revision 1.78
diff -u -p -r1.78 sshconnect1.c
--- sshconnect1.c   15 Nov 2015 22:26:49 -  1.78
+++ sshconnect1.c   18 Sep 2016 19:04:30 -
@@ -504,7 +504,6 @@ ssh_kex(char *host, struct sockaddr *hos
u_char cookie[8];
u_int supported_ciphers;
u_int server_flags, client_flags;
-   u_int32_t rnd = 0;
 
debug("Waiting for server public key.");
 
@@ -563,12 +562,7 @@ ssh_kex(char *host, struct sockaddr *hos
 * random number, interpreted as a 32-byte key, with the least
 * significant 8 bits being the first byte of the key.
 */
-   for (i = 0; i < 32; i++) {
-   if (i % 4 == 0)
-   rnd = arc4random();
-   session_key[i] = rnd & 0xff;
-   rnd >>= 8;
-   }
+   arc4random_buf(session_key, SSH_SESSION_KEY_LENGTH);
 
/*
 * According to the protocol spec, the first byte of the session key



Re: teach BFD how to send route messages, again

2016-09-18 Thread Peter Hessler
On 2016 Sep 17 (Sat) at 13:15:56 +0200 (+0200), Peter Hessler wrote:
:On 2016 Sep 17 (Sat) at 04:50:29 -0600 (-0600), Theo de Raadt wrote:
::>  route(8) for the ramdisks is not built with SMALL, so adding SMALL
::> won't help.
::
::Then you'll help it.   By compiling them SMALL.
:
:Happilly.
:
:Tested on an amd64 bsd.rd. dhclient, route add, route del, route show,
:all still work, and it's slightly smaller.
:

All options, commands, modifiers, and special names for networks have
been tested.  Some modifiers are already disabled in a SMALL kernel (esp
mpath and mpls).


:textdatabss dec hex
:213397  889636896   259189  3f475 obj/route
:212677  889636896   258469  3f1a5 obj/route-SMALL
:
:
:OK?
:
:Index: distrib/special/route/Makefile
:===
:RCS file: /cvs/openbsd/src/distrib/special/route/Makefile,v
:retrieving revision 1.1
:diff -u -p -u -p -r1.1 Makefile
:--- distrib/special/route/Makefile 23 Dec 2014 17:16:03 -  1.1
:+++ distrib/special/route/Makefile 17 Sep 2016 11:09:59 -
:@@ -4,7 +4,7 @@ PROG=  route
: MAN=  route.8
: SRCS= route.c show.c
: 
:-CFLAGS+=  -Wall
:+CFLAGS+=  -Wall -DSMALL
: 
: route.o .depend lint tags: keywords.h
: 
:

-- 
If A = B and B = C, then A = C, except where void or prohibited by law.
-- Roy Santoro



netinet/tcp_input.c syn_cache_get diff

2016-09-18 Thread David Hill
Hello -

Make sure we keep TF_NOPUSH set if TCP_NOPUSH was set.

FreeBSD has the same:
https://github.com/freebsd/freebsd/blob/c9af4f2541fd437e0805365fbeec46d69e033310/sys/netinet/tcp_syncache.c#L860
 
Index: netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.327
diff -u -p -r1.327 tcp_input.c
--- netinet/tcp_input.c 15 Sep 2016 02:00:18 -  1.327
+++ netinet/tcp_input.c 18 Sep 2016 17:07:09 -
@@ -3798,7 +3798,7 @@ syn_cache_get(struct sockaddr *src, stru
(void) m_free(am);
 
tp = intotcpcb(inp);
-   tp->t_flags = sototcpcb(oso)->t_flags & TF_NODELAY;
+   tp->t_flags = sototcpcb(oso)->t_flags & (TF_NOPUSH|TF_NODELAY);
if (sc->sc_request_r_scale != 15) {
tp->requested_s_scale = sc->sc_requested_s_scale;
tp->request_r_scale = sc->sc_request_r_scale;



Re: Manpage for uctuctl(4)

2016-09-18 Thread Ingo Schwarze
Hi Rafael,

Rafael Neves wrote on Sun, Sep 18, 2016 at 12:29:35PM +0100:
> On Sun, Sep 18, 2016 at 03:33:00PM +0200, Ingo Schwarze wrote:

>>  - Put the correct manual page author into the Copyright notice.

> I think that I shouldn't be in the copyright notice, because thre
> is no original work from me. I just copied the dwctwo(4) manpage
> and tweaked it, it is why there is Visa name there.

Technically, what you sent is a *derived work*.  In that case,
the original Copyright applies to the unchanged parts, and new
Copyright comes into existence covering your changes, so in
general, there should be two Copyright lines with different names.

However, diffing the two files, i find that all that remains from
the original file is this:

 .Os
 .Sh NAME
 .Sh SYNOPSIS
 .Sh DESCRIPTION
 The
 .Nm
 driver provides support for ...
 devices.
 .Sh SEE ALSO
 .Xr ehci 4 ,
 .Xr ohci 4
 .Sh HISTORY
 The
 .Nm
 driver first appeared in

That is all boilerplate text, imho insufficient to establish
Copyright, and besides, Visa explicitly confirmed that he does not
recognize the file as containing any of his work any longer, after
your changes.  If you delete all original work from a file, you can
delete the Copyright notice as well.  On the other hand, adding
your Copyright makes sense because you changed and added various
lines of text containing actual content.  So if the file is worthy
of Copyright at all - which i think it is, creativity standards in
Copyright are quite low - your name should be there.  And even if
the file as whole would not meet the creativity threshold, putting
your Copyright header is better than having none because it avoids
doubt.

Do you still object?

> I think it is like when you copy a source file and tweaks some
> magic numbers, or use a whole file in some other place in the tree
> with some modifications. It generally does not implies putting the
> name in the copyright notice, what I think is correct.

For minor changes in a substantial file, you are right.  But in
this case, non-boilerplate Copyrightable content is sparse in the
first place, and you changed most of what there is.

Yours,
  Ingo



Re: Manpage for uctuctl(4)

2016-09-18 Thread Visa Hankala
On Sun, Sep 18, 2016 at 03:33:00PM +0200, Ingo Schwarze wrote:
> So, here is a cleaned-up version:
> 
>  - Move the new page to the proper directory.
>  - Mention it in the Makefile.
>  - Put the correct manual page author into the Copyright notice.
>  - Add the architecture to the .Dt line.
>  - Remove the needless .Pq from the .Cd line
>(it might be useful in ehci(4), but not here).
>  - Remove the argument from .Nm in the DESCRIPTION.
>  - Append an AUTHORS section.
> 
> In long and complicated manuals, the AUTHORS section can also contain
> a sentence like:  This manual page was written by Rafael Neves.
> But i don't think that's interesting for such a short manual.
> 
> OK?
>   Ingo

ok visa@



Re: random malloc junk

2016-09-18 Thread Otto Moerbeek
On Wed, Sep 14, 2016 at 05:41:55AM +0200, Theo Buehler wrote:

> On Tue, Sep 13, 2016 at 01:29:17PM +0200, Otto Moerbeek wrote:
> > As often, real life came in between. Did anybody do measurements? I
> > really would like to to see hard data.
> 
> It seems that the price is relatively modest. 
> 
> I ran several 'make build's:
> 
> 3rd gen X1, i7 5500U, 2.4GHz (amd64):
>   no malloc.conf, malloc.c r1.195:
>1525.04 real  2505.98 user  1362.47 sys
>   no malloc.conf, malloc.c + diff:
>1532.15 real  2540.63 user  1356.98 sys
> 
>   for comparison:
>   malloc.conf -> J, malloc.c + diff:
>1554.76 real  2596.40 user  1353.26 sys
> 
> Acer Aspire 5633WLMi, Core 2 Duo 1.66 GHz (i386):
>   no malloc.conf, malloc.c, r1.195:
>5020.77 real  5725.21 user  1609.28 sys
>   no malloc.conf, malloc.c + diff:
>5088.07 real  5865.80 user  1572.12 sys

Did some more thinking about this. Since this diff interferes with the
effort to move canary bytes closer to the end of the requested size
(See tedu's diff from December
https://marc.info/?l=openbsd-tech=144966528402282=2), I like to
revisit that diff and fix it, and then see if and how we can to random
junking. 

tb@ pointed out at least realloc is broken with that diff. I have to
go and see if I can spot the actual problem.

-Otto






Re: merge ping6(8) into ping(8)

2016-09-18 Thread Theo de Raadt
> On Sun, Sep 18, 2016 at 12:11 AM, Theo de Raadt  wrote:
> >>  > this does 2 things:
> >>  > [...]
> >>
> >> I may recall what I have sent to you in private email, excerpt from
> >> FreeBSD ping6 manpage:
> >> [...]
> >> When we have two binaries I have more trust when one of them is working
> >> *only* with IPv4 and another one *only* with IPv6.
> >>
> >> So, what user problems are you trying to solve with this merge?
> >>
> >
> > Mikhail, with great regret I am informing you that the opinion of some
> > random gmail user does not actually matter around here.
> 
> It's not about opinion, everyone has it. It's about understanding why
> a thing was done. If a person can't answer to the question "why" a
> thing was done, I suppose he don't fully understand a solution to a
> problem he is trying to solve.
> 
> Random gmail users can bring value as much as fullnamed developers.

sorry to hear you don't understand

this is not really a forum to attempt education



Re: Manpage for uctuctl(4)

2016-09-18 Thread Rafael Neves
Hi,

On Sun, Sep 18, 2016 at 03:33:00PM +0200, Ingo Schwarze wrote:
> Hi,
> 
> Mike Belopuhov wrote on Sun, Sep 18, 2016 at 01:35:45PM +0200:
> > On Sun, Sep 18, 2016 at 06:15 +, Visa Hankala wrote:
> >> On Sat, Sep 17, 2016 at 02:40:09PM +0100, Rafael Neves wrote:
> 
> >>> Here follows a manpage for octuctl(4), based on dwctwo(4) manpage.
> >>> I am not sure if the title should stop in Controller or in Interface,
> >>> so I mantained the phrasing of commit.
> 
> I don't know either, so i left your wording untouched.
>
> >> My name does not belong to the copyright because I am not the author
> >> of this work.
> >> Is it customary to write manual pages for driver glue?
> 
> > Traditionally, you'd add a line "echi* at octuctl?" to echi(4)
> > man page with a cross reference to octuctl(4) in the SEE ALSO
> > section
> 
> Given that that isn't done for obio(4) on landisk and socppc either,
> that octuctl(4/octeon) is MD, and that .Xrs from MI to MD pages
> are slightly awkward, i'd maybe not put anything into ehci(4).
> Correct me if you think that's wrong, i know little about drivers.
> 
> > and then add the description of what is octuctl into
> > it's own man page.
> > 
> > Just like usb(4) mentions dwctwo? attachment and then dwctwo(4)
> > describes what is it.
> 
> So, here is a cleaned-up version:
> 
>  - Move the new page to the proper directory.
Agreed.
>  - Mention it in the Makefile.
Agreed.
>  - Put the correct manual page author into the Copyright notice.
I think that I shouldn't be in the copyright notice, because thre is no 
original work from me. I just copied the dwctwo(4) manpage and tweaked it, it 
is why there is Visa name there.

I think it is like when you copy a source file and tweaks some magic numbers, 
or use a whole file in some other place in the tree with some modifications. It 
generally does not implies putting the name in the copyright notice, what I 
think is correct.

>  - Add the architecture to the .Dt line.
Agreed.
>  - Remove the needless .Pq from the .Cd line
>(it might be useful in ehci(4), but not here).
Agreed, thanks!
>  - Remove the argument from .Nm in the DESCRIPTION.
Agreed, thanks!
>  - Append an AUTHORS section.
Agreed.
> 
> In long and complicated manuals, the AUTHORS section can also contain
> a sentence like:  This manual page was written by Rafael Neves.
> But i don't think that's interesting for such a short manual.
Agreed. And in this specific case, I have just tweaked an existing manpage.
> 
> OK?
>   Ingo
>
[snip] 



video(1): Don't limit maximum frame size to display size

2016-09-18 Thread Gregor Best
Hi,

this is another patch to video(1). It removes the logic that forces the
video grab frame size to be <= the size of the Xv adaptor. This allows
me to grab and view frames that are 1280x1024 on my 1440x900 screen. Xv
properly scales and stretches the window content, so there's nothing
that gets cut off from the video feed.

The camera I have here has the following resolutions:

160x120: 30
176x144: 30
320x240: 30
352x288: 30
640x480: 30
1280x1024: 9

Without this patch, the maximum resolution I can grab and view at the
same time is 640x480, which is a bit too tiny. I haven't noticed any
downsides of the patch, but I've only tested it with my laptop and not
other graphics cards, so I'm not sure how Xv handles frames larger than
the display size there.

-- 
Gregor

Index: video.c
===
RCS file: /mnt/media/cvs/xenocara/app/video/video.c,v
retrieving revision 1.19
diff -u -p -r1.19 video.c
--- video.c 6 Jun 2016 19:31:22 -   1.19
+++ video.c 18 Sep 2016 14:00:06 -
@@ -1162,13 +1162,6 @@ choose_size(struct video *vid)
else if (vid->width && !vid->height)
vid->height = vid->width * 3 / 4;
 
-   if (vid->mode & M_OUT_XV) {
-   if (vid->width > x->max_width)
-   vid->width = x->max_width;
-   if (vid->height > x->max_height)
-   vid->height = x->max_height;
-   }
-
if (vid->mode & M_IN_DEV) {
i = 0;
while (i < d->nsizes &&



Re: Manpage for uctuctl(4)

2016-09-18 Thread Ingo Schwarze
Hi,

Mike Belopuhov wrote on Sun, Sep 18, 2016 at 01:35:45PM +0200:
> On Sun, Sep 18, 2016 at 06:15 +, Visa Hankala wrote:
>> On Sat, Sep 17, 2016 at 02:40:09PM +0100, Rafael Neves wrote:

>>> Here follows a manpage for octuctl(4), based on dwctwo(4) manpage.
>>> I am not sure if the title should stop in Controller or in Interface,
>>> so I mantained the phrasing of commit.

I don't know either, so i left your wording untouched.

>> My name does not belong to the copyright because I am not the author
>> of this work.
>> Is it customary to write manual pages for driver glue?

> Traditionally, you'd add a line "echi* at octuctl?" to echi(4)
> man page with a cross reference to octuctl(4) in the SEE ALSO
> section

Given that that isn't done for obio(4) on landisk and socppc either,
that octuctl(4/octeon) is MD, and that .Xrs from MI to MD pages
are slightly awkward, i'd maybe not put anything into ehci(4).
Correct me if you think that's wrong, i know little about drivers.

> and then add the description of what is octuctl into
> it's own man page.
> 
> Just like usb(4) mentions dwctwo? attachment and then dwctwo(4)
> describes what is it.

So, here is a cleaned-up version:

 - Move the new page to the proper directory.
 - Mention it in the Makefile.
 - Put the correct manual page author into the Copyright notice.
 - Add the architecture to the .Dt line.
 - Remove the needless .Pq from the .Cd line
   (it might be useful in ehci(4), but not here).
 - Remove the argument from .Nm in the DESCRIPTION.
 - Append an AUTHORS section.

In long and complicated manuals, the AUTHORS section can also contain
a sentence like:  This manual page was written by Rafael Neves.
But i don't think that's interesting for such a short manual.

OK?
  Ingo


Index: Makefile
===
RCS file: /cvs/src/share/man/man4/man4.octeon/Makefile,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile
--- Makefile17 Nov 2015 13:25:36 -  1.4
+++ Makefile18 Sep 2016 13:16:14 -
@@ -1,6 +1,6 @@
 #  $OpenBSD: Makefile,v 1.4 2015/11/17 13:25:36 visa Exp $
 
-MAN=   amdcf.4 cnmac.4 octrng.4 octrtc.4
+MAN=   amdcf.4 cnmac.4 octrng.4 octrtc.4 octuctl.4
 MANSUBDIR=octeon
 
 .include 
Index: octuctl.4
===
RCS file: octuctl.4
diff -N octuctl.4
--- /dev/null   1 Jan 1970 00:00:00 -
+++ octuctl.4   18 Sep 2016 13:16:14 -
@@ -0,0 +1,48 @@
+.\"$OpenBSD$
+.\"
+.\" Copyright (c) 2016 Rafael Neves 
+.\"
+.\" Permission to use, copy, modify, and distribute this software for any
+.\" purpose with or without fee is hereby granted, provided that the above
+.\" copyright notice and this permission notice appear in all copies.
+.\"
+.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+.\"
+.Dd $Mdocdate$
+.Dt OCTUCTL 4 octeon
+.Os
+.Sh NAME
+.Nm octuctl
+.Nd Octeon II USB Controller Interface
+.Sh SYNOPSIS
+.Cd "octuctl0 at iobus? irq 56"
+.Cd "ehci0at octuctl?"
+.Cd "ohci0at octuctl?"
+.Sh DESCRIPTION
+The
+.Nm
+driver provides support for the Octeon II USB Controller Interface,
+which provides an interface to
+.Xr ehci 4
+and
+.Xr ohci 4
+devices.
+.Sh SEE ALSO
+.Xr ehci 4 ,
+.Xr ohci 4
+.Sh HISTORY
+The
+.Nm
+driver first appeared in
+.Ox 6.0 .
+.Sh AUTHORS
+The
+.Nm
+driver was written by
+.An Jonathan Matthew Aq Mt jmatt...@openbsd.org .



Re: Manpage for uctuctl(4)

2016-09-18 Thread Mike Belopuhov
On Sun, Sep 18, 2016 at 06:15 +, Visa Hankala wrote:
> On Sat, Sep 17, 2016 at 02:40:09PM +0100, Rafael Neves wrote:
> > Hi, 
> > 
> > Here follows a manpage for octuctl(4), based on dwctwo(4) manpage.
> > I am not sure if the title should stop in Controller or in Interface,
> > so I mantained the phrasing of commit.
> > 
> > Index: share/man/man4/octuctl.4
> > ===
> > RCS file: share/man/man4/octuctl.4
> > diff -N share/man/man4/octuctl.4
> > --- /dev/null   1 Jan 1970 00:00:00 -
> > +++ share/man/man4/octuctl.47 Sep 2016 21:55:30 -
> > @@ -0,0 +1,43 @@
> > +.\"$OpenBSD: octuctl.4,v 1.3 2016/02/02 17:38:20 jmc Exp $
> > +.\"
> > +.\" Copyright (c) 2016 Visa Hankala
> > +.\"
> > +.\" Permission to use, copy, modify, and distribute this software for any
> > +.\" purpose with or without fee is hereby granted, provided that the above
> > +.\" copyright notice and this permission notice appear in all copies.
> > +.\"
> > +.\" THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL 
> > WARRANTIES
> > +.\" WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> > +.\" MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> > +.\" ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> > +.\" WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> > +.\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> > +.\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > +.\"
> > +.Dd $Mdocdate: July 28 2016 $
> > +.Dt OCTUCTL 4
> > +.Os
> > +.Sh NAME
> > +.Nm octuctl
> > +.Nd Octeon II USB Controller Interface
> > +.Sh SYNOPSIS
> > +.Cd "octuctl0 at iobus? irq 56" Pq "octeon"
> > +.Cd "ehci0at octuctl?"
> > +.Cd "ohci0at octuctl?"
> > +.Sh DESCRIPTION
> > +The
> > +.Nm octuctl
> > +driver provides support for the Octeon II USB Controller Interface,
> > +which provides an interface to
> > +.Xr ehci 4
> > +and
> > +.Xr ohci 4
> > +devices.
> > +.Sh SEE ALSO
> > +.Xr ehci 4 ,
> > +.Xr ohci 4
> > +.Sh HISTORY
> > +The
> > +.Nm
> > +driver first appeared in
> > +.Ox 6.0 .
> 
> My name does not belong to the copyright because I am not the author
> of this work.
> 
> Is it customary to write manual pages for driver glue?
> 

Traditionally, you'd add a line "echi* at octuctl?" to echi(4)
man page with a cross reference to octuctl(4) in the SEE ALSO
section and then add the description of what is octuctl into
it's own man page.

Just like usb(4) mentions dwctwo? attachment and then dwctwo(4)
describes what is it.



Re: arm libunwind

2016-09-18 Thread Patrick Wildt
On Sun, Sep 18, 2016 at 08:16:42AM +0200, Mark Kettenis wrote:
> This makes libunwind build on OpenBSD/arm.  And it even works if you
> compile the code that uses exceptions with the right compiler options.
> 
> ok?

ok patrick@

> 
> Index: src/AddressSpace.hpp
> ===
> RCS file: /cvs/src/lib/libunwind/src/AddressSpace.hpp,v
> retrieving revision 1.3
> diff -u -p -r1.3 AddressSpace.hpp
> --- src/AddressSpace.hpp  5 Sep 2016 18:49:19 -   1.3
> +++ src/AddressSpace.hpp  18 Sep 2016 06:11:29 -
> @@ -35,7 +35,7 @@ namespace libunwind {
>  #include "Registers.hpp"
>  
>  #if _LIBUNWIND_ARM_EHABI
> -#if defined(__FreeBSD__) || defined(__NetBSD__)
> +#if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
>  
>  typedef void *_Unwind_Ptr;
>  
> 



Re: arm assembly syntax

2016-09-18 Thread Jonathan Gray
On Sun, Sep 18, 2016 at 12:04:48PM +0200, Mark Kettenis wrote:
> Somewhere down the line the ARM people made some subtle changes to
> their assembly syntax.  The new syntax is called Unified Assembler
> Language (UAL), and tis is what clang supports.  But gas defaults to
> the old pre-UAL dialect.  The easiest way to make code compile with
> both clang and gcc is to tell gas to switch into UAL mode.  The
> following diff does this for the one file in libc where it matters.
> The resulting binary code is identical.
> 
> ok?

We already do this for some files in the kernel

./arch/arm/arm/in_cksum_arm.S:.syntax unified
./lib/libkern/arch/arm/memcpy.S:.syntax unified
./lib/libkern/arch/arm/memset.S:.syntax unified

ok jsg@

> 
> 
> Index: lib/libc/arch/arm/string/ffs.S
> ===
> RCS file: /cvs/src/lib/libc/arch/arm/string/ffs.S,v
> retrieving revision 1.5
> diff -u -p -r1.5 ffs.S
> --- lib/libc/arch/arm/string/ffs.S6 Aug 2016 19:16:09 -   1.5
> +++ lib/libc/arch/arm/string/ffs.S18 Sep 2016 09:53:30 -
> @@ -44,6 +44,8 @@
>   * 16 Feb 1994.
>   */
>  
> + .syntax unified
> +
>  ENTRY(ffs)
>   /* Standard trick to isolate bottom bit in r0 or 0 if r0 = 0 on entry */
>   rsb r1, r0, #0
> @@ -58,7 +60,7 @@ ENTRY(ffs)
>   rsbne   r0, r0, r0, lsl #16 /* r0 = X * 0x0450fbaf */
>
>   /* now lookup in table indexed on top 6 bits of r0 */
> - ldrneb  r0, [ r2, r0, lsr #26 ]
> + ldrbne  r0, [ r2, r0, lsr #26 ]
>  
>  mov   pc, lr
>  END_WEAK(ffs)
> 



arm assembly syntax

2016-09-18 Thread Mark Kettenis
Somewhere down the line the ARM people made some subtle changes to
their assembly syntax.  The new syntax is called Unified Assembler
Language (UAL), and tis is what clang supports.  But gas defaults to
the old pre-UAL dialect.  The easiest way to make code compile with
both clang and gcc is to tell gas to switch into UAL mode.  The
following diff does this for the one file in libc where it matters.
The resulting binary code is identical.

ok?


Index: lib/libc/arch/arm/string/ffs.S
===
RCS file: /cvs/src/lib/libc/arch/arm/string/ffs.S,v
retrieving revision 1.5
diff -u -p -r1.5 ffs.S
--- lib/libc/arch/arm/string/ffs.S  6 Aug 2016 19:16:09 -   1.5
+++ lib/libc/arch/arm/string/ffs.S  18 Sep 2016 09:53:30 -
@@ -44,6 +44,8 @@
  * 16 Feb 1994.
  */
 
+   .syntax unified
+
 ENTRY(ffs)
/* Standard trick to isolate bottom bit in r0 or 0 if r0 = 0 on entry */
rsb r1, r0, #0
@@ -58,7 +60,7 @@ ENTRY(ffs)
rsbne   r0, r0, r0, lsl #16 /* r0 = X * 0x0450fbaf */
   
/* now lookup in table indexed on top 6 bits of r0 */
-   ldrneb  r0, [ r2, r0, lsr #26 ]
+   ldrbne  r0, [ r2, r0, lsr #26 ]
 
 mov   pc, lr
 END_WEAK(ffs)



Re: merge ping6(8) into ping(8)

2016-09-18 Thread Stuart Henderson
On 2016/09/18 00:04, Mikhail wrote:
> > this does 2 things:
> > [...]
> 
> I may recall what I have sent to you in private email, excerpt from FreeBSD
> ping6 manpage:
> 
> 8<8<8<8<8<8<
> There have been many discussions on why we separate ping6 and ping(8).
> Some people argued that it would be more convenient to uniform the ping
> command for both IPv4 and IPv6.  The followings are an answer to the
> request.
> 
> From a developer's point of view: since the underling raw sockets API is
> totally different between IPv4 and IPv6, we would end up having two types of
> code base.  There would actually be less benefit to uniform the two commands
> into a single command from the developer's standpoint.

Much of the code doesn't relate to the sockets API. Option parsing,
timestamp perturbation, printing, etc are all mostly common.

> From an operator's point of view: unlike ordinary network applications like
> remote login tools, we are usually aware of address family when using
> network management tools.  We do not just want to know the
> reachability to the host, but want to know the reachability to the host
> via a particular network protocol such as IPv6.  Thus, even if we had a
> unified ping(8) command for both IPv4 and IPv6, we would usually type a
> -6 or -4 option (or something like those) to specify the particular
> address family.  This essentially means that we have two different
> commands.
> 
> 8<8<8<8<8<8<
> 
> When we have two binaries I have more trust when one of them is working
> *only* with IPv4 and another one *only* with IPv6.

As a network operator if I'm interested in reachability to a specific
machine I'm certainly not going to rely on DNS.

And as a user if I'm interested in reachability of a service (which
covers the majority of ping usage) I don't care which af is used. In
fact I'd prefer to know about "whichever af is used by default to reach
this host".

> So, what user problems are you trying to solve with this merge?

Future changes to ping/ping6 only need to be done in one place, no risk
of them getting out of sync. Less disk space. Faster compiles. Less
setuid root code to audit.

The only reason I see for *not* merging them is that no developer
wanted to do the work.

Why should they be separate programs? We don't have to choose between
firefox and firefox6.



Re: merge ping6(8) into ping(8)

2016-09-18 Thread Mikhail
On Sun, Sep 18, 2016 at 12:11 AM, Theo de Raadt  wrote:
>>  > this does 2 things:
>>  > [...]
>>
>> I may recall what I have sent to you in private email, excerpt from
>> FreeBSD ping6 manpage:
>> [...]
>> When we have two binaries I have more trust when one of them is working
>> *only* with IPv4 and another one *only* with IPv6.
>>
>> So, what user problems are you trying to solve with this merge?
>>
>
> Mikhail, with great regret I am informing you that the opinion of some
> random gmail user does not actually matter around here.

It's not about opinion, everyone has it. It's about understanding why
a thing was done. If a person can't answer to the question "why" a
thing was done, I suppose he don't fully understand a solution to a
problem he is trying to solve.

Random gmail users can bring value as much as fullnamed developers.