Re: regarding OpenSSL License change

[Tom Cosgrove]

>>> Jimmy Hess 27-Mar-17 02:49 >>>
:
> silence does not generally grant permission. 

Since never grants permission.

> But the people in that project might be able to convincingly deliver some
> kind of argument that they've had implicit or "understood" permissions
> made at time of submission to use contributions however the project
> collectively agrees to use them.

Absolutely not.

When I contribute to an open source project, I do so under the terms of the
licences in the files I work on _at that time_.  If I completely rewrite or
add new files, I put those files under the standard licence used by the project,
and that code is then licenced in that (possibly different) way.

And the specific licence is important to me.  It is a significant factor in the
choice of which project to work on (which is why I choose to hack on OpenBSD
rather than, say, Linux).

The terms under which I contribute are those licences - there is no other
implied permission.  If anyone wants to change the licence used by code I have
contributed, they need my approval.  And if they want me to be accommodating,
there had better be a public discussion about alternative licences first.

Tom

Re: regarding OpenSSL License change

[Jimmy Hess]

> > From: "Constantine A. Murenin" 
> > If we do not hear from you, we will assume that you have no objection.
> Is this for real?!
> Who do they think they are?  ...
>People should not bother to respond to such nonsense, and then sue
> OpenSSL for obvious copyright infringement

I think "Don't bother to respond, and plan to sue" would be a poor
response,  that would just hurt everyone involved.Of course
silence does not generally grant permission. But the people in
that project might be able to convincingly deliver some kind of
argument that they've had implicit or "understood"  permissions made
at time of submission to use contributions however the project
collectively agrees to use them.


I think it would be most helpful if say  Three or Four  significant
contributors would either  Object / Say No  on the basis  of
disapproving  of  the  "Change procedure"  Or get their lawyers to
draft a Cease & Decist,  On behalf of both themself and their
co-authors,  based on the implied intent to infringe.

And also,  Go remind those folksthat distributed Binaries based on
OpenSSL tree will be infringing with a changed license document  if
Even 1 Contributor  has not agreed to the re-license.

Also, there is no work-around for a contributor denying.   They might
have the  idea of simply Removing and Replacing a  contribution  (Even
if you can accurately identify and rewrite specific lines of code from
a certain author)  does not  necessarily make the distribution
Non-infringing,   As  later code is likely to have built on top of
earlier code.


A suggested concept would be contributors  Replying  to the inquiry with
something firmly saying No,  and  reminding them that Derivative works
include non-literal copying.

EG  [EXAMPLE ] language:

"I do not approve of the manner in which this license change is
being negotiated;  All my co-authors/co-contributors to this code
base must explicitly agree to the change in principle for me to consider
granting permission.

I Do Not consent at this time to any license change regarding
any part of any of my submitted or committed code, Nor any modified version
or derivative work of my contribution(s) created by non-literal copying
of my work deviating from the terms of the the OpenSSL+SSLeay license
documents found in the source tree at the time that my contribution
was made.

If a license statement was not included with any work I submitted, then
my default terms are: Copyright, All Rights Reserved.

I hereby pre-emptively remind you that:

Derivative work includes all code added to the project, even by
other developers that followed my contributions in time which
extended any functionality on top of OpenSSL based on changing
or extending my earlier work, or related to my code in any way,
Including design style, naming conventions, usage of headers
and function prototypes, variable names, and miscellaneous
aesthetic qualities of my contributions.

Please recall the following text from the SSLeay license terms
which applies to my contributions and all OpenSSL project code based
on SSLeay:

 * The licence and distribution terms for any publically available version or
 * derivative of this code cannot be changed.  i.e. this code cannot simply be
 * copied and put under another distribution licence
 * [including the GNU Public Licence.]
"



--
-JH

Re: RTC clocks

[Todd C. Miller]

On Sun, 26 Mar 2017 23:25:48 +0200, Mark Kettenis wrote:

> The downside of this is that it becomes impossible to set the time
> back that far.  But the upside is that if you haven't left a machine
> powered off for a very long time, the ntpd constraint will actually
> work.

I think that is a reasonable trade off.

 - todd

RTC clocks

[Mark Kettenis]

A lot of the armv7/arm64 boards do not have a battery powered RTC.
Some of these boards actually do have an RTC, but if the board loses
power, the RTC resets itself.  If you then power up the board again,
it will trust the time in the RTC and you find yourself back in 1970.

The diff below adds a sanity check to the sxirtc(4) driver.  If the
year read back from the chip is the first year that can be represented
by the RTC, it considers the time to be invalid.  It will then take
the time from the filesystem and print:

  WARNING: preposterous clock chip time
  WARNING: CHECK AND RESET THE DATE!

The downside of this is that it becomes impossible to set the time
back that far.  But the upside is that if you haven't left a machine
powered off for a very long time, the ntpd constraint will actually
work.

ok?

Index: dev/fdt/sxirtc.c
===
RCS file: /cvs/src/sys/dev/fdt/sxirtc.c,v
retrieving revision 1.1
diff -u -p -r1.1 sxirtc.c
--- dev/fdt/sxirtc.c21 Jan 2017 08:26:49 -  1.1
+++ dev/fdt/sxirtc.c26 Mar 2017 21:12:23 -
@@ -149,7 +149,8 @@ sxirtc_gettime(todr_chip_handle_t handle
if (dt.dt_sec > 59 || dt.dt_min > 59 ||
dt.dt_hour > 23 || dt.dt_wday > 6 ||
dt.dt_day > 31 || dt.dt_day == 0 ||
-   dt.dt_mon > 12 || dt.dt_mon == 0)
+   dt.dt_mon > 12 || dt.dt_mon == 0 ||
+   dt.dt_year <= sc->base_year)
return 1;
 
tv->tv_sec = clock_ymdhms_to_secs();

Re: sendsyslog file race

[Alexander Bluhm]

On Sun, Mar 26, 2017 at 05:00:12PM +0200, Mateusz Guzik wrote:
> The patch is somewhat incorrect, although from what I checked it happens
> to generate the expected outcome in terms of assembly (the global pointer
> read only once and then a local copy accessed several times). You either
> need a linux-like READ_ONCE macros which casts through a volatile pointer
> or mark the global as volatile to prevent the compiler from issuing
> multiple reads in the future.

Note that our OpenBSD kernel is still implemented with a big kernel
lock in most places.  So multi processor thinking and locking is
not necessary.  The execution order can only be interrupted by
hardware interrups or explicitly rescheduling with tsleep(9).  Here
the sleep is hidden in copyin(), mallocarray(), sosend(), malloc(),
ktrgenio().

Interrupts are not relevant, they never change syslogf.  As tsleep()
is a function call, it automatically acts as compiler barrier.  So
volatile is not needed.

> Remaining ses sof syslogf are also super fishy:
> 1. logclose
> 2. logioctl -> LIOCSFD
> FRELE(syslogf, p);

A few months ago I would have said FRELE() does not sleep.  I think
this is currently still the case as we do not grab the netlock for
socketpairs.  But we did for a while.

As we are heading towards multi processor in kernel, I would suggest
this diff.  It orders FREF() and FRELE() in a way that we only
operate on refcounted global variables.  Although not necessary
with the current sleeping points, I think it results in more robust
code.

ok?

bluhm

Index: kern/subr_log.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/subr_log.c,v
retrieving revision 1.49
diff -u -p -r1.49 subr_log.c
--- kern/subr_log.c 24 Mar 2017 16:42:38 -  1.49
+++ kern/subr_log.c 26 Mar 2017 19:37:05 -
@@ -172,10 +172,12 @@ logopen(dev_t dev, int flags, int mode, 
 int
 logclose(dev_t dev, int flag, int mode, struct proc *p)
 {
+   struct file *fp;
 
-   if (syslogf)
-   FRELE(syslogf, p);
+   fp = syslogf;
syslogf = NULL;
+   if (fp)
+   FRELE(fp, p);
log_open = 0;
logsoftc.sc_state = 0;
return (0);
@@ -355,11 +357,11 @@ logioctl(dev_t dev, u_long com, caddr_t 
case LIOCSFD:
if ((error = suser(p, 0)) != 0)
return (error);
-   if ((error = getsock(p, *(int *)data, )) != 0)
+   fp = syslogf;
+   if ((error = getsock(p, *(int *)data, )) != 0)
return (error);
-   if (syslogf)
-   FRELE(syslogf, p);
-   syslogf = fp;
+   if (fp)
+   FRELE(fp, p);
break;
 
default:
Index: kern/uipc_syscalls.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.150
diff -u -p -r1.150 uipc_syscalls.c
--- kern/uipc_syscalls.c14 Feb 2017 09:46:21 -  1.150
+++ kern/uipc_syscalls.c26 Mar 2017 19:37:05 -
@@ -1146,8 +1146,8 @@ getsock(struct proc *p, int fdes, struct
return (EBADF);
if (fp->f_type != DTYPE_SOCKET)
return (ENOTSOCK);
-   *fpp = fp;
FREF(fp);
+   *fpp = fp;
 
return (0);
 }

recallocarray in tail(1)

[Martijn van Duren]

Hello,

There's one instance in tail(1) where recallocarray is appropriate. In
fact, it would have prevented the regression I introduced originally.

OK?

martijn@

Index: read.c
===
RCS file: /cvs/src/usr.bin/tail/read.c,v
retrieving revision 1.18
diff -u -p -r1.18 read.c
--- read.c  1 Feb 2017 16:21:12 -   1.18
+++ read.c  26 Mar 2017 16:29:24 -
@@ -166,10 +166,9 @@ lines(struct tailfile *tf, off_t off)
if (recno >= lineno) {
nlineno = (lineno + 1024) > off ?
(size_t) off : lineno + 1024;
-   lines = reallocarray(lines, nlineno, sizeof(*lines));
-   if (lines == NULL)
+   if ((lines = recallocarray(lines, lineno, nlineno,
+   sizeof(*lines))) == NULL)
err(1, NULL);
-   bzero(lines + recno, nlineno - lineno);
lineno = nlineno;
}
if (ch == '\n') {

Re: vmd fix

[Mike Larkin]

On Sun, Mar 26, 2017 at 04:40:03PM +0200, Mark Kettenis wrote:
> There is still a bit of an issue after the last set of changes made by
> mlarkin@.  The changed get_input_data() interface takes a pointer to a
> uint32_t as an argument, but only modifies the bytes that correspond
> to the access size.  That means that if we read the value into an
> uint32_t that is allocated on the stack, because if the access size is
> less than 4 bytes, we end up with stack garbage in the variable.  This
> is a problem in the mc146818 emulation code.
> 
> The result is that seabios (sometimes) detects the wrong memory size
> and subsequently triggers the following kernel printf:
> 
>   unknown memory type 1 for GPA 0x207bffd0
> 
> Not sure what happens with the VM at that point.  It seems to be
> hanging.
> 
> Diff below fixes the issue.  As far as I can see the i8253 and i8259
> emulation code isn't affected as the uint32_t stack variable gets
> converted into a uint8_t before being used.  But perhaps we should
> initialize the stack variables there as well to prevent further
> accidents.
> 
> ok?
> 

Yep, go for it

> 
> Index: mc146818.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/mc146818.c,v
> retrieving revision 1.10
> diff -u -p -r1.10 mc146818.c
> --- mc146818.c25 Mar 2017 22:36:53 -  1.10
> +++ mc146818.c26 Mar 2017 14:26:10 -
> @@ -249,7 +249,7 @@ vcpu_exit_mc146818(struct vm_run_params 
>   union vm_exit *vei = vrp->vrp_exit;
>   uint16_t port = vei->vei.vei_port;
>   uint8_t dir = vei->vei.vei_dir;
> - uint32_t data;
> + uint32_t data = 0;
>  
>   get_input_data(vei, );
>  
> 

Re: sendsyslog file race

[Mateusz Guzik]

On Fri, Mar 24, 2017 at 4:56 PM, Alexander Bluhm 
wrote:

> Hi,
>
> There is a race in dosendsyslog() which resulted in a crash on a
> 5.9 system.  sosend(syslogf->f_data, ...) was called with a NULL
> pointer.  So syslogf is not NULL, f_data is NULL and f_count is 1.
>
> The file structure is ref counted, but the global variable syslogf
> is not protected.  So it may change during sleep and dosendsyslog()
> possibly uses a different socket at each access.
>
> Solution is to access syslogf ony once, use a local copy, and do
> the ref counting there.
>

I'm sending this from the gmail web interface, so formatting may be screwed
up. Apologies in advance.

The patch is somewhat incorrect, although from what I checked it happens
to generate the expected outcome in terms of assembly (the global pointer
read only once and then a local copy accessed several times). You either
need a linux-like READ_ONCE macros which casts through a volatile pointer
or mark the global as volatile to prevent the compiler from issuing
multiple reads in the future.

Remaining ses sof syslogf are also super fishy:

1. logclose

if (syslogf)
FRELE(syslogf, p);
syslogf = NULL;

If this results in fdrop I presume it can block. But if that happens, the
global points to a defunct object.

2. logioctl -> LIOCSFD

if (syslogf)
FRELE(syslogf, p);
syslogf = fp;

This one will give double free and ref leaks.

Suggested fix is to xchg the pointer. While it is more expensive than
necessary for this case, it does not pose a problem. Also xchg is
blatantly obvious, while manual replacement would require explicit memory
barriers to be placed here.

vmd fix

[Mark Kettenis]

There is still a bit of an issue after the last set of changes made by
mlarkin@.  The changed get_input_data() interface takes a pointer to a
uint32_t as an argument, but only modifies the bytes that correspond
to the access size.  That means that if we read the value into an
uint32_t that is allocated on the stack, because if the access size is
less than 4 bytes, we end up with stack garbage in the variable.  This
is a problem in the mc146818 emulation code.

The result is that seabios (sometimes) detects the wrong memory size
and subsequently triggers the following kernel printf:

  unknown memory type 1 for GPA 0x207bffd0

Not sure what happens with the VM at that point.  It seems to be
hanging.

Diff below fixes the issue.  As far as I can see the i8253 and i8259
emulation code isn't affected as the uint32_t stack variable gets
converted into a uint8_t before being used.  But perhaps we should
initialize the stack variables there as well to prevent further
accidents.

ok?


Index: mc146818.c
===
RCS file: /cvs/src/usr.sbin/vmd/mc146818.c,v
retrieving revision 1.10
diff -u -p -r1.10 mc146818.c
--- mc146818.c  25 Mar 2017 22:36:53 -  1.10
+++ mc146818.c  26 Mar 2017 14:26:10 -
@@ -249,7 +249,7 @@ vcpu_exit_mc146818(struct vm_run_params 
union vm_exit *vei = vrp->vrp_exit;
uint16_t port = vei->vei.vei_port;
uint8_t dir = vei->vei.vei_dir;
-   uint32_t data;
+   uint32_t data = 0;
 
get_input_data(vei, );
 

/dev/sound

[Jan Stary]

Now that /dev/sound is gone,
should AUDIO_DEV_SOUND be removed from audio.c ?

Jan



Index: audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.161
diff -u -p -r1.161 audio.c
--- audio.c 11 Mar 2017 10:12:45 -  1.161
+++ audio.c 26 Mar 2017 11:40:52 -
@@ -50,7 +50,6 @@
 #define DEVNAME(sc)((sc)->dev.dv_xname)
 #define AUDIO_UNIT(n)  (minor(n) & 0x0f)
 #define AUDIO_DEV(n)   (minor(n) & 0xf0)
-#define AUDIO_DEV_SOUND0   /* minor of /dev/sound0 */
 #define AUDIO_DEV_MIXER0x10/* minor of /dev/mixer0 */
 #define AUDIO_DEV_AUDIO0x80/* minor of /dev/audio0 */
 #define AUDIO_DEV_AUDIOCTL 0xc0/* minor of /dev/audioctl */
@@ -1137,7 +1136,6 @@ audio_detach(struct device *self, int fl
 * close uses device_lookup, it returns EXIO and does nothing
 */
mn = self->dv_unit;
-   vdevgone(maj, mn | AUDIO_DEV_SOUND, mn | AUDIO_DEV_SOUND, VCHR);
vdevgone(maj, mn | AUDIO_DEV_AUDIO, mn | AUDIO_DEV_AUDIO, VCHR);
vdevgone(maj, mn | AUDIO_DEV_AUDIOCTL, mn | AUDIO_DEV_AUDIOCTL, VCHR);
vdevgone(maj, mn | AUDIO_DEV_MIXER, mn | AUDIO_DEV_MIXER, VCHR);
@@ -1608,7 +1606,6 @@ audioopen(dev_t dev, int flags, int mode
error = ENXIO;
else {
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
error = audio_open(sc, flags);
break;
@@ -1634,7 +1631,6 @@ audioclose(dev_t dev, int flags, int ifm
if (sc == NULL)
return ENXIO;
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
error = audio_close(sc);
break;
@@ -1659,7 +1655,6 @@ audioread(dev_t dev, struct uio *uio, in
if (sc == NULL)
return ENXIO;
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
error = audio_read(sc, uio, ioflag);
break;
@@ -1684,7 +1679,6 @@ audiowrite(dev_t dev, struct uio *uio, i
if (sc == NULL)
return ENXIO;
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
error = audio_write(sc, uio, ioflag);
break;
@@ -1709,7 +1703,6 @@ audioioctl(dev_t dev, u_long cmd, caddr_
if (sc == NULL)
return ENXIO;
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
error = audio_ioctl(sc, cmd, addr);
break;
@@ -1744,7 +1737,6 @@ audiopoll(dev_t dev, int events, struct 
if (sc == NULL)
return POLLERR;
switch (AUDIO_DEV(dev)) {
-   case AUDIO_DEV_SOUND:
case AUDIO_DEV_AUDIO:
revents = audio_poll(sc, events, p);
break;

Re: dsdt: redundant assignment

[Jonathan Gray]

On Sun, Mar 26, 2017 at 09:33:44AM +0200, Otto Moerbeek wrote:
> On Sun, Mar 26, 2017 at 06:31:41PM +1100, Jonathan Gray wrote:
> 
> > On Sun, Mar 26, 2017 at 09:14:26AM +0200, Anton Lindqvist wrote:
> > > Hi,
> > > An assignment introduced in r1.219 looks redundant.
> > 
> > child is assigned every iteration of the loop this diff looks wrong to me.
> 
> It's being assigned in the if statement.
> 
>   -Otto

Ah indeed.

> > 
> > > 
> > > Index: dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.231
> > > diff -u -p -r1.231 dsdt.c
> > > --- dsdt.c16 Feb 2017 18:02:22 -  1.231
> > > +++ dsdt.c25 Mar 2017 21:16:04 -
> > > @@ -1261,7 +1261,6 @@ aml_find_node(struct aml_node *node, con
> > >   const char *nn;
> > >  
> > >   SIMPLEQ_FOREACH(child, >son, sib) {
> > > - nn = child->name;
> > >   if ((nn = child->name) != NULL) {
> > >   if (*nn == AMLOP_ROOTCHAR) nn++;
> > >   while (*nn == AMLOP_PARENTPREFIX) nn++;
> > > 
> 

Re: dsdt: redundant assignment

[Otto Moerbeek]

On Sun, Mar 26, 2017 at 06:31:41PM +1100, Jonathan Gray wrote:

> On Sun, Mar 26, 2017 at 09:14:26AM +0200, Anton Lindqvist wrote:
> > Hi,
> > An assignment introduced in r1.219 looks redundant.
> 
> child is assigned every iteration of the loop this diff looks wrong to me.

It's being assigned in the if statement.

-Otto
> 
> > 
> > Index: dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.231
> > diff -u -p -r1.231 dsdt.c
> > --- dsdt.c  16 Feb 2017 18:02:22 -  1.231
> > +++ dsdt.c  25 Mar 2017 21:16:04 -
> > @@ -1261,7 +1261,6 @@ aml_find_node(struct aml_node *node, con
> > const char *nn;
> >  
> > SIMPLEQ_FOREACH(child, >son, sib) {
> > -   nn = child->name;
> > if ((nn = child->name) != NULL) {
> > if (*nn == AMLOP_ROOTCHAR) nn++;
> > while (*nn == AMLOP_PARENTPREFIX) nn++;
> > 

Re: dsdt: redundant assignment

[Jonathan Gray]

On Sun, Mar 26, 2017 at 09:14:26AM +0200, Anton Lindqvist wrote:
> Hi,
> An assignment introduced in r1.219 looks redundant.

child is assigned every iteration of the loop this diff looks wrong to me.

> 
> Index: dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.231
> diff -u -p -r1.231 dsdt.c
> --- dsdt.c16 Feb 2017 18:02:22 -  1.231
> +++ dsdt.c25 Mar 2017 21:16:04 -
> @@ -1261,7 +1261,6 @@ aml_find_node(struct aml_node *node, con
>   const char *nn;
>  
>   SIMPLEQ_FOREACH(child, >son, sib) {
> - nn = child->name;
>   if ((nn = child->name) != NULL) {
>   if (*nn == AMLOP_ROOTCHAR) nn++;
>   while (*nn == AMLOP_PARENTPREFIX) nn++;
> 

dsdt: redundant assignment

[Anton Lindqvist]

Hi,
An assignment introduced in r1.219 looks redundant.

Index: dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.231
diff -u -p -r1.231 dsdt.c
--- dsdt.c  16 Feb 2017 18:02:22 -  1.231
+++ dsdt.c  25 Mar 2017 21:16:04 -
@@ -1261,7 +1261,6 @@ aml_find_node(struct aml_node *node, con
const char *nn;
 
SIMPLEQ_FOREACH(child, >son, sib) {
-   nn = child->name;
if ((nn = child->name) != NULL) {
if (*nn == AMLOP_ROOTCHAR) nn++;
while (*nn == AMLOP_PARENTPREFIX) nn++;