Re: ld.so: shared objects with same-name global function symbol

2015-05-07 Thread Philip Guenther
On Thu, May 7, 2015 at 5:42 PM, Sviatoslav Chagaev
 wrote:
...
> If we read the dynamic symbol tables of both libraries, here's what we
> see:
>
> $ readelf -sD libaa/libaa.so.1.0 libbb/libbb.so.1.0 | grep dup
>31  34: 0bc0 3FUNC GLOBAL DEFAULT   8 
> duplicateFun
>35  34: 0d10 6FUNC GLOBAL DEFAULT   8 
> duplicateFun
>
> Both symbols are of "function" type and "global" binding.
>
> Here's what SysV ABI [3] has to say regarding this about relocatable
> objects:
>
> When the link editor combines several relocatable object files,
> it does not allow multiple definitions of STB_GLOBAL symbols with the
> same name.
>
> But I think it makes sense to not allow this for shared objects too.
> How can one expect a program to work correctly if there is a
> collision between two shared objects which is silently ignored? This
> also seems like an illegitimate way to replace some functionality of
> another lib with your own...

This happens often enough to have a name: interposition, and must not
be considered a fatal error.


> Do you agree that when there are shared objects which have global-bound
> function symbols with the same name -- ld.so must not allow such a
> program to run/dlopen said shared objects?

I disagree.  To quote the Solaris Linkers and Libraries Guide:
Another form of simple symbol resolution, interposition, occurs
between relocatable objects
and shared objects, or between multiple shared objects. In these
cases, when a symbol is
multiply-defined, the relocatable object, or the first definition
between multiple shared objects,
is silently taken by the link-editor. The relocatable object's
definition, or the first shared object's
definition, is said to interpose on all other definitions. This
interposition can be used to override
the functionality provided by another shared object.
Multiply-defined symbols that occur
between relocatable objects and shared objects, or between
multiple shared objects, are treated
identically. A symbols weak binding or global binding is
irrelevant. By resolving to the first
definition, regardless of the symbols binding, both the
link-editor and runtime linker behave
consistently.

That's not an extension; that's just the way the linker works.


Philip Guenther



Re: vfs_shutdown would like to do polled I/O at least on panic

2015-05-07 Thread Masao Uebayashi
By doing complex VFS shutdown operation, the system's memory image will
be modified a lot since a panic was triggered.  I'd totally skip
vfs_shutdown() after a panic [1], then do the best to dump a kernel core
for analysis.

[1] OpenBSD's panic(9) sets RB_NOSYNC only when panicstr is already set
(== recursive panic()) at this moment.



ld.so: shared objects with same-name global function symbol

2015-05-07 Thread Sviatoslav Chagaev
Hi,

I'm investigating Alf Schlichting's report [1] of regression in ld.so
caused by my diff [2]. This regression reproduces on amd64-current too.

I found errors in my diff which break the dlclose/test1/prog2 test. But
what's really interesting -- is the dlclose/test1/prog3 test.

I found that with current ld.so, dlclose/test1/prog3 works because the
libaa and libbb are loaded as a "load group" because there is an
explicit dependency from libaa to libbb. If, however, the dependency
between shared objects is removed, like in the diff below -- the test
crashes both with and without my diff:

===> dlclose/test1/prog4
./prog4
*** Signal SIGSEGV in dlclose/test1/prog4 (:48 
'run-regress-prog4')
FAILED
*** Error 1 in target 'regress' (ignored)


If we read the dynamic symbol tables of both libraries, here's what we
see:

$ readelf -sD libaa/libaa.so.1.0 libbb/libbb.so.1.0 | grep dup
   31  34: 0bc0 3FUNC GLOBAL DEFAULT   8 
duplicateFun
   35  34: 0d10 6FUNC GLOBAL DEFAULT   8 
duplicateFun

Both symbols are of "function" type and "global" binding.

Here's what SysV ABI [3] has to say regarding this about relocatable
objects:

When the link editor combines several relocatable object files,
it does not allow multiple definitions of STB_GLOBAL symbols with the
same name.

But I think it makes sense to not allow this for shared objects too.
How can one expect a program to work correctly if there is a
collision between two shared objects which is silently ignored? This
also seems like an illegitimate way to replace some functionality of
another lib with your own...

Do you agree that when there are shared objects which have global-bound
function symbols with the same name -- ld.so must not allow such a
program to run/dlopen said shared objects?

[1] http://marc.info/?l=openbsd-ports&m=142901624010155&w=2
[2] http://marc.info/?l=openbsd-bugs&m=141653930524559&w=2
[3] http://www.sco.com/developers/gabi/latest/ch4.symtab.html


Index: regress/libexec/ld.so/dlclose//test1/Makefile
===
RCS file: /cvs/src/regress/libexec/ld.so/dlclose/test1/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- regress/libexec/ld.so/dlclose//test1/Makefile   30 Sep 2005 15:16:18 
-  1.2
+++ regress/libexec/ld.so/dlclose//test1/Makefile   7 May 2015 23:33:48 
-
@@ -1,5 +1,5 @@
 # $OpenBSD: Makefile,v 1.2 2005/09/30 15:16:18 kurt Exp $
 
-SUBDIR+= libbb libaa prog1 prog2 prog3
+SUBDIR+= libbb libaa libcc prog1 prog2 prog3 prog4
 
 .include 
Index: regress/libexec/ld.so/dlclose//test1/Makefile.inc
===
RCS file: /cvs/src/regress/libexec/ld.so/dlclose/test1/Makefile.inc,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 Makefile.inc
--- regress/libexec/ld.so/dlclose//test1/Makefile.inc   28 Sep 2005 15:42:32 
-  1.1.1.1
+++ regress/libexec/ld.so/dlclose//test1/Makefile.inc   7 May 2015 23:33:48 
-
@@ -17,3 +17,12 @@ BB_OBJDIR!=  if [ -d $(BB_DIR)/${__objdir
else \
echo "$(BB_DIR)"; \
fi
+
+CC_DIR=${.CURDIR}/../libcc
+
+CC_OBJDIR!=if [ -d $(CC_DIR)/${__objdir} ]; then \
+   echo "$(CC_DIR)/${__objdir}"; \
+   else \
+   echo "$(CC_DIR)"; \
+   fi
+
--- /dev/null   Fri May  8 02:41:30 2015
+++ regress/libexec/ld.so/dlclose/test1/libcc/Makefile  Mon Apr 20 00:12:26 2015
@@ -0,0 +1,9 @@
+#  $OpenBSD: Makefile,v 1.1.1.1 2005/09/28 15:42:32 kurt Exp $
+
+LIB=   cc
+SRCS=  cc.c
+LDADD+=-Wl,-E
+
+regress: all
+
+.include 
--- /dev/null   Fri May  8 02:41:30 2015
+++ regress/libexec/ld.so/dlclose/test1/libcc/cc.c  Mon Apr 20 00:10:51 2015
@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2005 Kurt Miller 
+ *
+ * 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.
+ *
+ */
+
+int ccSymbol;
+
+int
+duplicateFun()
+{
+   return (0);
+}
+
--- /dev/null   Fri May  8 02:41:30 2015
+++ regress/libexec/ld.so/dlclose/test1/libcc/shlib_version Mon Apr 20 
00:11:10 2015
@@ -0,0 +1,2 @@
+major=1
+minor=0 
--- /dev/null   Fri May  8 02:41:41 201

dupfdopen() api modification

2015-05-07 Thread Vitaliy Makkoveev
In the not far future fd_getfile() will return the referenced file
instance, so dupfdopen() should be modified for the same reasons
that getsock() and getvnode().

The modified dupfdopen() receives a thread pointer instead of it's
file descriptor table and of it's file descriptor for duplication.
The thread pointer, passed to dupfdopen() and "current" thread pointer
used within dupfdopen() are the same, so current was replaced by
passed thread pointer. Thread struct has "p_dupfd" field which is
descriptor for duplication, so it is used instead of removed "dfd" arg.

Index: sys/kern/kern_descrip.c
===
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.119
diff -u -p -r1.119 kern_descrip.c
--- sys/kern/kern_descrip.c 30 Apr 2015 21:18:45 -  1.119
+++ sys/kern/kern_descrip.c 7 May 2015 23:22:23 -
@@ -1225,8 +1225,9 @@ filedescopen(dev_t dev, int mode, int ty
  * Duplicate the specified descriptor to a free descriptor.
  */
 int
-dupfdopen(struct filedesc *fdp, int indx, int dfd, int mode)
+dupfdopen(struct proc *p, int indx, int mode)
 {
+   struct filedesc *fdp = p->p_fd;
struct file *wfp;
 
fdpassertlocked(fdp);
@@ -1235,10 +1236,10 @@ dupfdopen(struct filedesc *fdp, int indx
 * Assume that the filename was user-specified; applications do
 * not tend to open /dev/fd/# when they can just call dup()
 */
-   if ((curproc->p_p->ps_flags & (PS_SUGIDEXEC | PS_SUGID))) {
-   if (curproc->p_descfd == 255)
+   if ((p->p_p->ps_flags & (PS_SUGIDEXEC | PS_SUGID))) {
+   if (p->p_descfd == 255)
return (EPERM);
-   if (curproc->p_descfd != curproc->p_dupfd)
+   if (p->p_descfd != p->p_dupfd)
return (EPERM);
}
 
@@ -1249,7 +1250,7 @@ dupfdopen(struct filedesc *fdp, int indx
 * because fd_getfile will return NULL if the file at indx is
 * newly created by falloc (FIF_LARVAL).
 */
-   if ((wfp = fd_getfile(fdp, dfd)) == NULL)
+   if ((wfp = fd_getfile(fdp, p->p_dupfd)) == NULL)
return (EBADF);
 
/*
@@ -1263,7 +1264,7 @@ dupfdopen(struct filedesc *fdp, int indx
 
fdp->fd_ofiles[indx] = wfp;
fdp->fd_ofileflags[indx] = (fdp->fd_ofileflags[indx] & UF_EXCLOSE) |
-   (fdp->fd_ofileflags[dfd] & ~UF_EXCLOSE);
+   (fdp->fd_ofileflags[p->p_dupfd] & ~UF_EXCLOSE);
wfp->f_count++;
fd_used(fdp, indx);
return (0);
Index: sys/kern/vfs_syscalls.c
===
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.220
diff -u -p -r1.220 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 7 May 2015 08:53:33 -   1.220
+++ sys/kern/vfs_syscalls.c 7 May 2015 23:22:23 -
@@ -862,8 +862,7 @@ doopenat(struct proc *p, int fd, const c
if ((error = vn_open(&nd, flags, cmode)) != 0) {
if (error == ENODEV &&
p->p_dupfd >= 0 &&  /* XXX from fdopen */
-   (error =
-   dupfdopen(fdp, indx, p->p_dupfd, flags)) == 0) {
+   (error = dupfdopen(p, indx, flags)) == 0) {
closef(fp, p);
*retval = indx;
goto out;
Index: sys/sys/filedesc.h
===
RCS file: /cvs/src/sys/sys/filedesc.h,v
retrieving revision 1.30
diff -u -p -r1.30 filedesc.h
--- sys/sys/filedesc.h  6 May 2015 08:52:17 -   1.30
+++ sys/sys/filedesc.h  7 May 2015 23:22:23 -
@@ -121,7 +121,7 @@ struct filedesc0 {
  * Kernel global variables and routines.
  */
 void   filedesc_init(void);
-intdupfdopen(struct filedesc *, int, int, int);
+intdupfdopen(struct proc *, int, int);
 intfdalloc(struct proc *p, int want, int *result);
 void   fdexpand(struct proc *);
 intfalloc(struct proc *p, struct file **resultfp, int *resultfd);



Re: opensmtpd + acceptable mail

2015-05-07 Thread Giovanni Bechis
On 05/07/15 03:33, James Turner wrote:
> So I'm not quite sure how to explain this but I'm getting similiar
> emails to the one below and it seems like opensmtpd should be rejecting
> them as they don't seem like they are a valid format.
> 
> Have others seen emails like these? Should opensmtpd be rejecting them?
> 
I can recreate those wrong formatted emails with this .forward file, I remember 
it worked when there was sendmail.
 Giovanni



Prova.eml
Description: application/extension-eml
"| /usr/sbin/sendmail -f giova...@cvs.openbsd.org giova...@paclan.it"



Re: tcp keep-alives sent without timestamps

2015-05-07 Thread Mike Belopuhov
On 14 April 2015 at 21:08, Lauri Tirkkonen  wrote:
> On Tue, Apr 14 2015 20:40:58 +0200, Mike Belopuhov wrote:
>> According to 3.2 in RFC 7323:
>>
>>Once TSopt has been successfully negotiated, that is both  and
>> contain TSopt, the TSopt MUST be sent in every non-
>>segment for the duration of the connection, and SHOULD be sent in an
>> segment (see Section 5.2 for details).  The TCP SHOULD remember
>>this state by setting a flag, referred to as Snd.TS.OK, to one.  If a
>>non- segment is received without a TSopt, a TCP SHOULD silently
>>drop the segment.  A TCP MUST NOT abort a TCP connection because any
>>segment lacks an expected TSopt.
>
> Thank you, I somehow missed the existence of this RFC.
>
>> I had a stab at adding timestamp support to tcp_respond but couldn't
>> test yet.  If you feel like giving it a try, please be my guest.
>
> With your patch, I confirm that timestamps are present on keep-alive
> messages.
>

Hi Lauri,

I've committed the fix.  Thanks a lot for bringing this up and testing.



Re: tcp keep-alives sent without timestamps

2015-05-07 Thread Mike Belopuhov
On 6 May 2015 at 13:05, Martin Pieuchot  wrote:
> On 20/04/15(Mon) 18:37, Mike Belopuhov wrote:
>> On Tue, Apr 14, 2015 at 22:08 +0300, Lauri Tirkkonen wrote:
>> > On Tue, Apr 14 2015 20:40:58 +0200, Mike Belopuhov wrote:
>> > > According to 3.2 in RFC 7323:
>> > >
>> > >Once TSopt has been successfully negotiated, that is both  and
>> > > contain TSopt, the TSopt MUST be sent in every non-
>> > >segment for the duration of the connection, and SHOULD be sent in an
>> > > segment (see Section 5.2 for details).  The TCP SHOULD remember
>> > >this state by setting a flag, referred to as Snd.TS.OK, to one.  If a
>> > >non- segment is received without a TSopt, a TCP SHOULD silently
>> > >drop the segment.  A TCP MUST NOT abort a TCP connection because any
>> > >segment lacks an expected TSopt.
>> >
>> > Thank you, I somehow missed the existence of this RFC.
>> >
>>
>> Does anyone else want to comment on this?
>
> Diff looks good to me.  Since you mentioned other *BSD in your previous
> post, I look at what Linux and Solaris do and they both seem to set
> timestamps on keep alive packets.

Thanks.

> As for MD5 signatures Linux also include it.
>

Can't fix every problem with one patch...

> ok mpi@
>

Committed! Thanks for the review!



vfs_shutdown would like to do polled I/O at least on panic

2015-05-07 Thread Mike Belopuhov
As I've pointed out before, on panic we can be running on any
CPU and our disk controller's interrupts can interrupt on the
other one.  Since we'll most likely be holding a kernel lock,
dealing with unlocking it might get hairy very fast.  Instead
what we could do to improve the chances of a clean shutdown on
panic is to instruct our disk subsystem to do polled I/O that
will be run on the same CPU with the panic.

Initially I wanted to move "cold = 1" earlier in boot(), but
after talking to Miod, it started to look like a bad idea.

Thoughts?

diff --git sys/dev/ata/ata_wdc.c sys/dev/ata/ata_wdc.c
index 1f52488..aea9ec1 100644
--- sys/dev/ata/ata_wdc.c
+++ sys/dev/ata/ata_wdc.c
@@ -199,20 +199,22 @@ wd_hibernate_io(dev_t dev, daddr_t blkno, vaddr_t addr, 
size_t size, int op, voi
  */
 int
 wdc_ata_bio(struct ata_drive_datas *drvp, struct ata_bio *ata_bio)
 {
struct wdc_xfer *xfer;
struct channel_softc *chp = drvp->chnl_softc;
 
xfer = wdc_get_xfer(WDC_NOSLEEP);
if (xfer == NULL)
return WDC_TRY_AGAIN;
+   if (panicstr)
+   ata_bio->flags |= ATA_POLL;
if (ata_bio->flags & ATA_POLL)
xfer->c_flags |= C_POLL;
if (!(ata_bio->flags & ATA_POLL) &&
(drvp->drive_flags & (DRIVE_DMA | DRIVE_UDMA)) &&
(ata_bio->flags & ATA_SINGLE) == 0 &&
(ata_bio->bcount > 512 ||
(chp->wdc->quirks & WDC_QUIRK_NOSHORTDMA) == 0))
xfer->c_flags |= C_DMA;
xfer->drive = drvp->drive;
xfer->cmd = ata_bio;
diff --git sys/scsi/scsi_base.c sys/scsi/scsi_base.c
index 9cf6b45..3afcc29 100644
--- sys/scsi/scsi_base.c
+++ sys/scsi/scsi_base.c
@@ -1267,20 +1267,22 @@ scsi_report_luns(struct scsi_link *sc_link, int 
selectreport,
return (error);
 }
 
 void
 scsi_xs_exec(struct scsi_xfer *xs)
 {
xs->error = XS_NOERROR;
xs->resid = xs->datalen;
xs->status = 0;
CLR(xs->flags, ITSDONE);
+   if (panicstr)
+   SET(xs->flags, SCSI_AUTOCONF);
 
 #ifdef SCSIDEBUG
if (xs->sc_link->flags & SDEV_DB1) {
scsi_xs_show(xs);
if (xs->datalen && (xs->flags & SCSI_DATA_OUT))
scsi_show_mem(xs->data, min(64, xs->datalen));
}
 #endif
 
/* The adapter's scsi_cmd() is responsible for calling scsi_done(). */



Re: Code coverage report for LibreSSL tests

2015-05-07 Thread Brent Cook
On Wed, May 6, 2015 at 2:54 PM, Harri Porten  wrote:
> Hi!
>
> We've started to generate code coverage reports for test suites of some
> projects on a regular basis. You'll find on overview for LibreSSL (Portable)
> here:
>
>  http://www.opencoverage.net/libressl/index_html/sources.html
>
> Out of 59877 conditions and decisions 13389, or 22.36% were covered during
> yesterdays run. OpenSSL currently has 67832 conditions/decisions, i.e. the
> project's goal of simplifying the code has been reached. The percentage
> covered through the test suite is lower however. Hence I wonder whether our
> use of the Linux port or our build configuration is to blame.

Testing the latest LibreSSL 2.2.0 development branch, I now get the
following result testing just the libraries (libtls, libcrypto,
libssl) as it appears is reported at opencoverage.net:

Overall coverage rate:
  lines..: 39.6% (28500 of 72029 lines)
  functions..: 42.0% (2299 of 5472 functions)
  branches...: 31.8% (13420 of 42223 branches)

After adding the openssl(1) command itself, the overall coverage looks
like this:

Overall coverage rate:
  lines..: 33.5% (29595 of 88381 lines)
  functions..: 40.4% (2339 of 5795 functions)
  branches...: 26.4% (14017 of 53168 branches)

The test script I used to generate this is here:

https://github.com/libressl-portable/portable/blob/master/gen-coverage-report.sh

Here is an example where these stats can be a little misleading: a
compatibility wrapper that has largely been turned into a no-op on
purpose. In fact, we probably want to make sure that _no_ functions in
this file are called eventually:

http://www.opencoverage.net/libressl/index_html/source_332.html



Re: rev 1.37 of src/bin/cp/utils.c broke 'cp -p'

2015-05-07 Thread Stuart Henderson
Thanks for tracking this down, I've hit it during port builds but had
assumed it was dpb changes.

OK sthen@ to revert.


On 2015/05/07 14:29, David Coppa wrote:
> 
> Hi,
> 
> As found while running the git test-suite (for an upcoming update
> to git-2.4.0):
> 
> $ cd sub
> $ 
> rm -rf .git
> $ 
> cp -R -P -p ../.git/modules/sub .git
> cp: chflags: .git/refs/heads: Invalid argument
> cp: chflags: .git/refs/tags: Invalid argument
> cp: chflags: .git/refs/remotes/origin: Invalid argument
> cp: chflags: .git/refs/remotes: Invalid argument
> cp: chflags: .git/refs: Invalid argument
> cp: chflags: .git/branches: Invalid argument
> cp: chflags: .git/hooks: Invalid argument
> cp: chflags: .git/info: Invalid argument
> cp: chflags: .git/objects/pack: Invalid argument
> cp: chflags: .git/objects/info: Invalid argument
> cp: chflags: .git/objects/87: Invalid argument
> cp: chflags: .git/objects/d0: Invalid argument
> cp: chflags: .git/objects/91: Invalid argument
> cp: chflags: .git/objects/b2: Invalid argument
> cp: chflags: .git/objects: Invalid argument
> cp: chflags: .git/logs/refs/remotes/origin: Invalid argument
> cp: chflags: .git/logs/refs/remotes: Invalid argument
> cp: chflags: .git/logs/refs/heads: Invalid argument
> cp: chflags: .git/logs/refs: Invalid argument
> cp: chflags: .git/logs: Invalid argument
> cp: chflags: .git/: Invalid argument
> 
> Reverting to 1.36 makes cp work again as expected.
> 
> Ciao!
> David
> 



rev 1.37 of src/bin/cp/utils.c broke 'cp -p'

2015-05-07 Thread David Coppa

Hi,

As found while running the git test-suite (for an upcoming update
to git-2.4.0):

$ cd sub
$ rm 
-rf .git
$ cp 
-R -P -p ../.git/modules/sub .git
cp: chflags: .git/refs/heads: Invalid argument
cp: chflags: .git/refs/tags: Invalid argument
cp: chflags: .git/refs/remotes/origin: Invalid argument
cp: chflags: .git/refs/remotes: Invalid argument
cp: chflags: .git/refs: Invalid argument
cp: chflags: .git/branches: Invalid argument
cp: chflags: .git/hooks: Invalid argument
cp: chflags: .git/info: Invalid argument
cp: chflags: .git/objects/pack: Invalid argument
cp: chflags: .git/objects/info: Invalid argument
cp: chflags: .git/objects/87: Invalid argument
cp: chflags: .git/objects/d0: Invalid argument
cp: chflags: .git/objects/91: Invalid argument
cp: chflags: .git/objects/b2: Invalid argument
cp: chflags: .git/objects: Invalid argument
cp: chflags: .git/logs/refs/remotes/origin: Invalid argument
cp: chflags: .git/logs/refs/remotes: Invalid argument
cp: chflags: .git/logs/refs/heads: Invalid argument
cp: chflags: .git/logs/refs: Invalid argument
cp: chflags: .git/logs: Invalid argument
cp: chflags: .git/: Invalid argument

Reverting to 1.36 makes cp work again as expected.

Ciao!
David



Re: msleep(9) should handle cold and panics just like tsleep does

2015-05-07 Thread Mike Belopuhov
On Tue, May 05, 2015 at 11:02 -0700, Philip Guenther wrote:
> On Tue, May 5, 2015 at 9:35 AM, Mike Belopuhov  wrote:
> ...
> > Here's a diff to remedy this.  This is the same chunk as in the
> > tsleep, except it uses semantics of msleep.  IPL dance is there
> > to negate the IPL changing effect of mtx_enter/mtx_leave so that
> > splx(safepri) operation is actually changing IPL level from HIGH
> > to "safepri" and then back to the mutex IPL level.
> ...
> > +   spl = MUTEX_OLDIPL(mtx);
> > +   MUTEX_OLDIPL(mtx) = splhigh();
> > +   mtx_leave(mtx);
> > +
> > +   splx(safepri);
> 
> Hmm, since mtx_leave() effectively ends with
> "splx(MUTEX_OLDIPL(mtx));", can't we just say:
> 
>   spl = MUTEX_OLDIPL(mtx);
>   MUTEX_OLDIPL(mtx) = safepri;
>   mtx_leave(mtx);
> 
> (The tsleep() version needs splhigh() because that's the only safe way
> to get the current spl level so that it can be restored before
> returning.  Here in msleep() the mutex's MUTEX_OLDIPL() already has
> the value we need.)
> 
> 
> Philip Guenther

Sorry for the delay.  I have finally figured out why it's not enough
for MP (but still needs to go in).  The problem is that panic can run
on any CPU and that's where we're holding the kernel lock.  Interrupts
are happening on cpu0 and our lock prevents them from running.  It was
rather easy to demonstrate by dropping and re-acquiring the kernel lock
in msleep/tsleep.

I believe the easiest solution is to force SCSI_POLL and run all I/O
on the CPU we paniced on and that's what I'm currently investigating.

diff --git sys/kern/kern_synch.c sys/kern/kern_synch.c
index fbf7684..03308b4 100644
--- sys/kern/kern_synch.c
+++ sys/kern/kern_synch.c
@@ -140,11 +140,11 @@ tsleep(const volatile void *ident, int priority, const 
char *wmesg, int timo)
 
return (error);
 }
 
 /*
- * Same as tsleep, but if we have a mutex provided, then once we've 
+ * Same as tsleep, but if we have a mutex provided, then once we've
  * entered the sleep queue we drop the mutex. After sleeping we re-lock.
  */
 int
 msleep(const volatile void *ident, struct mutex *mtx, int priority,
 const char *wmesg, int timo)
@@ -153,16 +153,34 @@ msleep(const volatile void *ident, struct mutex *mtx, int 
priority,
int error, error1, spl;
 
KASSERT((priority & ~(PRIMASK | PCATCH | PNORELOCK)) == 0);
KASSERT(mtx != NULL);
 
+   if (cold || panicstr) {
+   /*
+* After a panic, or during autoconfiguration,
+* just give interrupts a chance, then just return;
+* don't run any other procs or panic below,
+* in case this is the idle process and already asleep.
+*/
+   spl = MUTEX_OLDIPL(mtx);
+   MUTEX_OLDIPL(mtx) = safepri;
+   mtx_leave(mtx);
+   if ((priority & PNORELOCK) == 0) {
+   mtx_enter(mtx);
+   MUTEX_OLDIPL(mtx) = spl;
+   } else
+   splx(spl);
+   return (0);
+   }
+
sleep_setup(&sls, ident, priority, wmesg);
sleep_setup_timeout(&sls, timo);
sleep_setup_signal(&sls, priority);
 
/* XXX - We need to make sure that the mutex doesn't
-* unblock splsched. This can be made a bit more 
+* unblock splsched. This can be made a bit more
 * correct when the sched_lock is a mutex.
 */
spl = MUTEX_OLDIPL(mtx);
MUTEX_OLDIPL(mtx) = splsched();
mtx_leave(mtx);



Re: opensmtpd + acceptable mail

2015-05-07 Thread Giovanni Bechis
On Wed, May 06, 2015 at 09:33:02PM -0400, James Turner wrote:
> So I'm not quite sure how to explain this but I'm getting similiar
> emails to the one below and it seems like opensmtpd should be rejecting
> them as they don't seem like they are a valid format.
> 
> Have others seen emails like these? Should opensmtpd be rejecting them?
> 
I can recreate those wrong formatted emails with this .forward file, I remember 
it worked when there was sendmail.
 Giovanni
Return-Path: 
Received: from localhost (localhost [127.0.0.1])
by thor.snb.it (Postfix) with ESMTP id 11D7F65F2
for ; Thu,  7 May 2015 09:31:52 +0200 (CEST)
X-Virus-Scanned: amavisd-new at snb.it
Received: from thor.snb.it ([127.0.0.1])
by localhost (thor.snb.it [127.0.0.1]) (amavisd-new, port 10026)
with ESMTP id bkNRubGaZFvt for ;
Thu,  7 May 2015 09:31:51 +0200 (CEST)
Received: from bigio.paclan.it (static-217-133-45-79.clienti.tiscali.it 
[217.133.45.79])
(Authenticated sender: s...@paclan.it)
by thor.snb.it (Postfix) with ESMTPSA id 614F865D0
for ; Thu,  7 May 2015 09:31:51 +0200 (CEST)
Received: from localhost (bigio.paclan.it [local])
by bigio.paclan.it (OpenSMTPD) with ESMTPA id ecaa384a
for ;
Thu, 7 May 2015 09:31:50 +0200 (CEST)

>From giova...@bigio.paclan.it Thu May  7 09: 31:50 2015
Return-Path: giova...@bigio.paclan.it
Delivered-To: giova...@bigio.paclan.it
Received: from localhost (bigio.paclan.it [local])
by bigio.paclan.it (OpenSMTPD) with ESMTPA id 0e403d24
for ;
Thu, 7 May 2015 09:31:50 +0200 (CEST)
From: Giovanni Bechis 
Date: Thu, 7 May 2015 09:31:50 +0200 (CEST)
Message-Id: <9173265272216522884.enqu...@bigio.paclan.it>
To: giova...@bigio.paclan.it
Subject: Prova

test
"| /usr/sbin/sendmail -f giova...@cvs.openbsd.org giova...@paclan.it"


process stuck in a sched_yield() loop

2015-05-07 Thread David Coppa

Can somebody with the necessary skills help me with this?

$ cd /usr/ports/devel/libinotify/ && make clean fake && make test 

I remember this used to work... Now, most of the times, the
'check_libinotify' process brings the CPU to 100% and it's stuck
in a sched_yield() loop:

 31456/1022146 check_libinotify RET   __thrwakeup 0
 31456/1022569 check_libinotify RET   __thrsleep 0
 31456/1022146 check_libinotify CALL  sigprocmask(SIG_BLOCK,0x8)
 31456/1000130 check_libinotify CALL  __thrwakeup(0x15cb19881800,1)
 31456/1022569 check_libinotify CALL  gettimeofday(0x15caa18674c0,0)
 31456/1022146 check_libinotify RET   sigprocmask 0<>
 31456/1022569 check_libinotify STRU  struct timeval { 1430986026<"May  7 
10:07:06 2015">.729702 }
 31456/1022146 check_libinotify CALL  sigprocmask(SIG_BLOCK,~0<>)
 31456/1000130 check_libinotify RET   __thrwakeup 0
 31456/1022569 check_libinotify RET   gettimeofday 0
 31456/1022146 check_libinotify RET   sigprocmask 0x8
 31456/1000130 check_libinotify CALL  sigprocmask(SIG_BLOCK,0x8)
 31456/1022569 check_libinotify CALL  gettimeofday(0x15caa18674c0,0)
 31456/1000130 check_libinotify RET   sigprocmask 0<>
 31456/1022569 check_libinotify STRU  struct timeval { 1430986026<"May  7 
10:07:06 2015">.729739 }
 31456/1027161 check_libinotify RET   __thrsleep 0
 31456/1022146 check_libinotify CALL  vfork()
 31456/1000130 check_libinotify CALL  
__thrsleep(0x15caad133000,CLOCK_REALTIME,0,0x15cac88bc640,0)
 31456/1022569 check_libinotify RET   gettimeofday 0
 31456/1027161 check_libinotify CALL  gettimeofday(0x15cb0aa9be50,0)
 31456/1022569 check_libinotify CALL  poll(0x15caa18675d0,1,2000)
 31456/1027161 check_libinotify STRU  struct timeval { 1430986026<"May  7 
10:07:06 2015">.729760 }
 31456/1027161 check_libinotify RET   gettimeofday 0
  2372/1002372 check_libinotify RET   vfork 0
 31456/1027161 check_libinotify CALL  gettimeofday(0x15cb0aa9be50,0)
 31456/1027161 check_libinotify STRU  struct timeval { 1430986026<"May  7 
10:07:06 2015">.730012 }
 31456/1027161 check_libinotify RET   gettimeofday 0
 31456/1027161 check_libinotify CALL  poll(0x15cb0aa9bf60,1,2000)
  2372/1002372 check_libinotify CALL  sigprocmask(SIG_SETMASK,0x8)
  2372/1002372 check_libinotify RET   sigprocmask ~0x10100
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotify CALL  sched_yield()
  2372/1002372 check_libinotify RET   sched_yield 0
  2372/1002372 check_libinotif

Introducing if_output()

2015-05-07 Thread Martin Pieuchot
This diff is a first step towards removing all pseudo-driver #ifdef
in ether_output().  As for ether_input() the goal of this work is to
provide an elegant design to make it easier to turn pseudo-drivers
MP-safe.

So instead of including some bridge(4), vlan(4) and carp(4) specific
code in ether_output(), I'd like to split this function and call the
interesting chunks in bridge_output(), vlan_output() and carp_output().

The first step is to take the generic code enqueuing packets in its
own function: if_output().

Sadly if_start() is still required for hfsc_deferred().

Comments, ok?

Index: net/bridgestp.c
===
RCS file: /cvs/src/sys/net/bridgestp.c,v
retrieving revision 1.52
diff -u -p -r1.52 bridgestp.c
--- net/bridgestp.c 14 Mar 2015 03:38:51 -  1.52
+++ net/bridgestp.c 28 Apr 2015 12:22:59 -
@@ -357,7 +357,6 @@ bstp_transmit_tcn(struct bstp_state *bs,
struct ifnet *ifp = bp->bp_ifp;
struct ether_header *eh;
struct mbuf *m;
-   int s, len, error;
 
if (ifp == NULL || (ifp->if_flags & IFF_RUNNING) == 0)
return;
@@ -382,16 +381,8 @@ bstp_transmit_tcn(struct bstp_state *bs,
bpdu.tbu_bpdutype = BSTP_MSGTYPE_TCN;
bcopy(&bpdu, mtod(m, caddr_t) + sizeof(*eh), sizeof(bpdu));
 
-   s = splnet();
bp->bp_txcount++;
-   len = m->m_pkthdr.len;
-   IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
-   if (error == 0) {
-   ifp->if_obytes += len;
-   ifp->if_omcasts++;
-   if_start(ifp);
-   }
-   splx(s);
+   if_output(ifp, m);
 }
 
 void
@@ -473,7 +464,7 @@ bstp_send_bpdu(struct bstp_state *bs, st
struct ifnet *ifp = bp->bp_ifp;
struct mbuf *m;
struct ether_header *eh;
-   int s, len, error;
+   int s;
 
s = splnet();
if (ifp == NULL || (ifp->if_flags & IFF_RUNNING) == 0)
@@ -521,13 +512,7 @@ bstp_send_bpdu(struct bstp_state *bs, st
m->m_pkthdr.pf.prio = BSTP_IFQ_PRIO;
 
bp->bp_txcount++;
-   len = m->m_pkthdr.len;
-   IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
-   if (error == 0) {
-   ifp->if_obytes += len;
-   ifp->if_omcasts++;
-   if_start(ifp);
-   }
+   if_output(ifp, m);
  done:
splx(s);
 }
Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.330
diff -u -p -r1.330 if.c
--- net/if.c23 Apr 2015 09:45:24 -  1.330
+++ net/if.c28 Apr 2015 12:22:59 -
@@ -421,7 +421,6 @@ if_attach_common(struct ifnet *ifp)
 void
 if_start(struct ifnet *ifp)
 {
-
splassert(IPL_NET);
 
if (ifp->if_snd.ifq_len >= min(8, ifp->if_snd.ifq_maxlen) &&
@@ -439,6 +438,35 @@ if_start(struct ifnet *ifp)
TAILQ_INSERT_TAIL(&iftxlist, ifp, if_txlist);
schednetisr(NETISR_TX);
}
+}
+
+int
+if_output(struct ifnet *ifp, struct mbuf *m)
+{
+   int s, error = 0;
+
+   s = splnet();
+
+   /*
+* Queue message on interface, and start output if interface
+* not yet active.
+*/
+   IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
+   if (error) {
+   splx(s);
+   return (error);
+   }
+
+   ifp->if_obytes += m->m_pkthdr.len;
+   if (m->m_flags & M_MCAST)
+   ifp->if_omcasts++;
+
+   ifp->if_opackets++;
+   if_start(ifp);
+
+   splx(s);
+
+   return (0);
 }
 
 struct mbuf_queue if_input_queue = MBUF_QUEUE_INITIALIZER(8192, IPL_NET);
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.235
diff -u -p -r1.235 if_bridge.c
--- net/if_bridge.c 17 Apr 2015 11:04:01 -  1.235
+++ net/if_bridge.c 28 Apr 2015 12:22:59 -
@@ -2683,7 +2683,6 @@ int
 bridge_ifenqueue(struct bridge_softc *sc, struct ifnet *ifp, struct mbuf *m)
 {
int error, len;
-   short mflags;
 
 #if NGIF > 0
/* Packet needs etherip encapsulation. */
@@ -2735,18 +2734,15 @@ bridge_ifenqueue(struct bridge_softc *sc
}
 #endif
len = m->m_pkthdr.len;
-   mflags = m->m_flags;
-   IFQ_ENQUEUE(&ifp->if_snd, m, NULL, error);
+
+   error = if_output(ifp, m);
if (error) {
sc->sc_if.if_oerrors++;
return (error);
}
+
sc->sc_if.if_opackets++;
sc->sc_if.if_obytes += len;
-   ifp->if_obytes += len;
-   if (mflags & M_MCAST)
-   ifp->if_omcasts++;
-   if_start(ifp);
 
return (0);
 }
Index: net/if_ethersubr.c
===
RCS file: /cvs/src/sys/net/if_ethersubr.c,v
retrieving revision 1.194
diff -u -p -r1.194 if_ethersubr.c
--- net/if_ethersubr.c  13 Apr 2015 08:52:51 -  1.194
+++ net/if_ethersubr.