a problem with pf's NAT...
Hi, i'm tesing the NAT with the pf on OpenBSD 5.8, but i can not make it successful.There is a server with pf having a internal IP 10.0.11.200 and external IP 61.xxx.xx.xx, then, i make a pf.conf with contents like below(having enable IP forwarding) : #my define int_if = "de1"#10.0.11.200 ext_if = "de2" #61.xxx.xx.xx int_net = "10.0.11.0/24" #my rules pass out on $ext_if inet from $int_if:network to any nat-to $ext_if after apply the config, i test working of the NAT from a client with IP 10.0.11.19 (who's gateway refer to 10.0.11.200), then i can not visit the external service such as a website, when i try to "netstat/n" on the client, get results as below: now, i need help for my problem, thanks for your reply. 55232E27@A5DDC838.6CA19D56 Description: Binary data
[calendar] Addition of a New Zealand holiday file
G'day, Similar to the recent British calendar file, here's a New Zealand file. I've nuked a few NZ items from calendar.holiday due to them being spelt wrongly, rigid dates, and are in the new file anyway. Like some holidays, there's a bit of historical controversy & emotion surrounding a couple, so I reckon this is a reasonable solution. Here are the main references I used: http://employment.govt.nz/er/holidaysandleave/publicholidays/publicholidaydates/current.asp http://en.wikipedia.org/wiki/Public_holidays_in_New_Zealand http://www.anzac.govt.nz/today/ http://en.wikipedia.org/wiki/ANZAC http://en.wikipedia.org/wiki/Anzac_Day http://en.wikipedia.org/wiki/New_Zealand_Day http://en.wikipedia.org/wiki/Dominion_Day Despite what is on a lot of web pages ANZAC is all upper case, due to it being an acronym for 'Australian and New Zealand Army Corps' - a remembrance day. Various Pacific islands celebrate it too, but the list changes, along with places of battle, and other parts of the Empire where lots of Aussies & Kiwis are currently living, such as London. 2 provincial anniversary days fall on the same day, and those 2 provinces are geographical neighbours, so put 1 entry for those days. The only problem was with the provincial Marlborough Anniversary Day, which is observed on the first Monday after Labour Day. I couldn't find any other entries for MonFith or if it would roll over to the first Monday of the next month when needed, so I left it on the *date. The 5 summer anniversary days from January flop about to the nearest Monday, forwards & backwards. so they are *dates. This began in a misc@ discussion: http://openbsd-archive.7691.n7.nabble.com/DIFF-New-Year-s-calendar-td286907.html Cheers! Index: calendar.1 === RCS file: /cvs/src/usr.bin/calendar/calendar.1,v retrieving revision 1.41 diff -u -p -r1.41 calendar.1 --- calendar.1 14 Jan 2016 20:08:01 - 1.41 +++ calendar.1 18 Jan 2016 21:47:34 - @@ -228,6 +228,8 @@ Jewish holidays (should be updated yearl so that roving holidays are set correctly for the current year). .It Pa calendar.music Musical events, births, and deaths (strongly oriented toward rock n' roll). +.It Pa calendar.nz +New Zealand calendar. .It Pa calendar.openbsd .Ox related events. Index: calendars/calendar.all === RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.all,v retrieving revision 1.6 diff -u -p -r1.6 calendar.all --- calendars/calendar.all 14 Jan 2016 20:08:01 - 1.6 +++ calendars/calendar.all 18 Jan 2016 21:47:34 - @@ -18,5 +18,6 @@ #include #include #include +#include #endif /* !_calendar_all_ */ Index: calendars/calendar.holiday === RCS file: /cvs/src/usr.bin/calendar/calendars/calendar.holiday,v retrieving revision 1.34 diff -u -p -r1.34 calendar.holiday --- calendars/calendar.holiday 14 Jan 2016 20:08:01 - 1.34 +++ calendars/calendar.holiday 18 Jan 2016 21:47:34 - @@ -44,7 +44,6 @@ 02/02 Candlemas 02/04 Independence Commemoration Day in Sri Lanka 02/05 Constitution Day in Mexico -02/06 New Zealand Day 02/07 Independence Day in Grenada 02/08 Preseren Day (Cultural Holiday) in Slovenia 02/09 St. Maron's Day in Lebanon @@ -137,7 +136,7 @@ 04/22 Oklahoma Day in Oklahoma 04/24 Victory Day in Togo 04/24* Pesach - First Day of Passover - Festival of Freedom -04/25 Anzac Day in Australia, New Zealand, Tonga, Western Samoa +04/25* ANZAC Day in Australia, New Zealand, and various other influenced places 04/25 Liberation Day in Italy 04/25 National Flag Day in Swaziland 04/26 Confederate Memorial Day in Florida & Georgia @@ -391,7 +390,6 @@ 10/23 Chulalongkron's Day in Thailand 10/24 Independence Day in Zambia 10/24 United Nations Day -10/25 Labor Day in New Zealand 10/25 Taiwan Restoration Day in Taiwan 10/26 Agam Day in Nauru 10/26 Armed Forces Day in Benin, Rwanda @@ -488,7 +486,6 @@ 06/02 Corpus Christi in Paraguay 06/MonFirstJefferson Davis's Birthday in Alabama & Mississippi (1st Monday) 06/MonFirstJefferson Davis's Birthday in Florida, Georgia, & S. Carolina -06/04 Queen's Birthday in New Zealand 06/06 His Majesty, Yang Di-Pertuan Agong's Birthday in Malaysia 06/11 Queen's Birthday 06/12 Peace with Bolivia in Paraguay --- /dev/null Mon Jan 18 21:47:42 2016 +++ calendars/calendar.nz Mon Jan 18 21:46:43 2016 @@ -0,0 +1,48 @@ +/* + * New Zealand holiday + * + * $OpenBSD$ + */ + +#ifndef _calendar_nz_ +#define _calendar_nz_ + +01/01 New Year's Day +01/02 Day after New Year's Day (public holiday) +02/06* New Zealand/Waitangi Day (public holiday) +02/14 Saint Valentine's Day +04/01 April Fools' Day +04/SunFirstDaylight Saving Time ends; clocks move back (first Sunday of April) +05/SunSecond Mother's Day (2nd Sunday in May) +06/MonFir
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 06:56:29PM +0100, Sebastien Marie wrote:
>
> Modulo the ENOTTY error code (see previous comment), yes the purpose is
> to early return from pledge_ioctl(). pledge(2) permits to expose only a
> portion of deeper kernel code for a set of defined operations.
>
Better diff: check for VBAD on the top of pledge_ioctl() so it
handles non-audio devices as well. Return ENOTTY, as would return
ioctl with no pledge().
OK?
Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.147
diff -u -p -u -p -r1.147 kern_pledge.c
--- kern_pledge.c 18 Jan 2016 17:19:55 - 1.147
+++ kern_pledge.c 18 Jan 2016 22:46:41 -
@@ -1150,8 +1150,11 @@ pledge_ioctl(struct proc *p, long com, s
}
/* fp != NULL was already checked */
- if (fp->f_type == DTYPE_VNODE)
+ if (fp->f_type == DTYPE_VNODE) {
vp = fp->f_data;
+ if (vp->v_type == VBAD)
+ return (ENOTTY);
+ }
/*
* Further sets of ioctl become available, but are checked a
Re: no more yp support in resolv.conf -> faq 10
Matthieu Herrb writes: > Support for 'yp' in resolv.conf was removed last november. > http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/asr/asr.c?rev=1.49&content-type=text/x-cvsweb-markup > > So don't mention it in the FAQ... > > ok ? Please don't remove the , else ok jca@ > Index: faq/faq10.html > === > RCS file: /cvs/www/faq/faq10.html,v > retrieving revision 1.206 > diff -u -p -u -r1.206 faq10.html > --- faq/faq10.html17 Jan 2016 03:32:03 - 1.206 > +++ faq/faq10.html18 Jan 2016 20:08:07 - > @@ -1727,17 +1727,6 @@ For details on selective group inclusion > http://www.openbsd.org/cgi-bin/man.cgi?query=group&sektion=5"; > >group(5). > > - > -If you are distributing hostnames via YP, you should now review > - href="http://www.openbsd.org/cgi-bin/man.cgi?query=resolv.conf&sektion=5"; > ->resolv.conf(5) and check that the name service lookup order is correct. > -Most users will require a line like this: > - > - > -lookup file yp bind > - > - > - > 10.20 - Character sets and localization > > OpenBSD uses the ASCII character set by default. It also supports -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
no more yp support in resolv.conf -> faq 10
Support for 'yp' in resolv.conf was removed last november. http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libc/asr/asr.c?rev=1.49&content-type=text/x-cvsweb-markup So don't mention it in the FAQ... ok ? Index: faq/faq10.html === RCS file: /cvs/www/faq/faq10.html,v retrieving revision 1.206 diff -u -p -u -r1.206 faq10.html --- faq/faq10.html 17 Jan 2016 03:32:03 - 1.206 +++ faq/faq10.html 18 Jan 2016 20:08:07 - @@ -1727,17 +1727,6 @@ For details on selective group inclusion http://www.openbsd.org/cgi-bin/man.cgi?query=group&sektion=5"; >group(5). - -If you are distributing hostnames via YP, you should now review -http://www.openbsd.org/cgi-bin/man.cgi?query=resolv.conf&sektion=5"; ->resolv.conf(5) and check that the name service lookup order is correct. -Most users will require a line like this: - - -lookup file yp bind - - - 10.20 - Character sets and localization OpenBSD uses the ASCII character set by default. It also supports -- Matthieu Herrb signature.asc Description: PGP signature
Re: ppp_tty uiomove() conversion
Martin Natano wrote:
> Below the uiomove() conversion for net/ppp_tty.c. M_TRAILINGSPACE()
> returns int, but the result can't be negative, so using u_int for the
> return value should be fine.
Looks good. m->m_len is unsigned and yes, given that the mbuf fields
aren't corrupted M_TRAILINGSPACE is >= 0.
ok?
> Index: net/ppp_tty.c
> ===
> RCS file: /cvs/src/sys/net/ppp_tty.c,v
> retrieving revision 1.41
> diff -u -p -u -r1.41 ppp_tty.c
> --- net/ppp_tty.c 21 Dec 2015 21:49:02 - 1.41
> +++ net/ppp_tty.c 13 Jan 2016 19:44:53 -
> @@ -321,7 +321,7 @@ pppread(struct tty *tp, struct uio *uio,
> splx(s);
>
> for (m = m0; m && uio->uio_resid; m = m->m_next)
> - if ((error = uiomovei(mtod(m, u_char *), m->m_len, uio)) != 0)
> + if ((error = uiomove(mtod(m, u_char *), m->m_len, uio)) != 0)
> break;
> m_freem(m0);
> return (error);
> @@ -336,7 +336,8 @@ pppwrite(struct tty *tp, struct uio *uio
> struct ppp_softc *sc = (struct ppp_softc *)tp->t_sc;
> struct mbuf *m, *m0, **mp;
> struct sockaddr dst;
> -int len, error;
> +u_int len;
> +int error;
>
> if ((tp->t_state & TS_CARR_ON) == 0 && (tp->t_cflag & CLOCAL) == 0)
> return 0; /* wrote 0 bytes */
> @@ -361,7 +362,7 @@ pppwrite(struct tty *tp, struct uio *uio
> len = M_TRAILINGSPACE(m);
> if (len > uio->uio_resid)
> len = uio->uio_resid;
> - if ((error = uiomovei(mtod(m, u_char *), len, uio)) != 0) {
> + if ((error = uiomove(mtod(m, u_char *), len, uio)) != 0) {
> m_freem(m0);
> return (error);
> }
>
> cheers,
> natano
>
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 06:56:29PM +0100, Sebastien Marie wrote: > On Mon, Jan 18, 2016 at 06:05:31PM +0100, Alexandre Ratchov wrote: > > On Mon, Jan 18, 2016 at 01:35:46PM +0100, Sebastien Marie wrote: > > > On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote: > > > > > > I am unsure about returning 0 for something we know is wrong to do. > > > > heh, it's not wrong; disconnecting audio devices is a legitimate > > use of them ;) > > I didn't want to said that the operation in a whole is wrong, but that > making pledge_ioctl() returning 0 would be wrong as we know the > operation shouldn't be allowed in normal case. the operation _is_ allowed in the normal case; except that the device is gone and the operation returns an error code indicating that it failed. > > > Isn't possible to return an error ? As example, calling > > > ioctl(TIOCGWINSZ) on no-tty device return ENOTTY. > > > > > > > Agreed, and it's same for chardevs: the ioctl syscall returns > > uncondionnaly ENOTTY on disconnected devices: sys_ioctl() calls > > vn_ioctl() which returns ENOTTY if vp->v_type == VBAD. > > hum ? I dunno if I was clear. Returning ENOTTY was an example. Does > disconnecting audio devices makes ioctl(2) return ENOTTY ? I doubt. ioctl() on any disconnected device returns ENOTTY, this is not specific to audio. > > Modulo the ENOTTY error code (see previous comment), yes the purpose is > to early return from pledge_ioctl(). pledge(2) permits to expose only a > portion of deeper kernel code for a set of defined operations. > OK, I dislike the ENOTTY error code as well. The most appropriate is EIO, as this is what read(2) and write(2) return in this case. (but in this case pledged code returns EIO, while non-pledged code returns ENOTTY. Not a big deal as the caller generally takes the same code-path code paths in both cases). does this looks better? Index: kern_pledge.c === RCS file: /cvs/src/sys/kern/kern_pledge.c,v retrieving revision 1.146 diff -u -p -u -p -r1.146 kern_pledge.c --- kern_pledge.c 9 Jan 2016 06:13:43 - 1.146 +++ kern_pledge.c 18 Jan 2016 18:23:09 - @@ -1205,10 +1205,13 @@ pledge_ioctl(struct proc *p, long com, s case AUDIO_GETENC: case AUDIO_SETFD: case AUDIO_GETPROPS: - if (fp->f_type == DTYPE_VNODE && - vp->v_type == VCHR && + if (fp->f_type != DTYPE_VNODE) + break; + if (vp->v_type == VCHR && cdevsw[major(vp->v_rdev)].d_open == audioopen) return (0); + if (vp->v_type == VBAD) + return (EIO); } #endif /* NAUDIO > 0 */ }
Re: spec_vnops.c uiomove() conversion
Martin Natano wrote:
> Below the conversion to uiomove() for kern/spec_vnops.c. This diff
> prevents truncation of uio_resid when passed to min().
Looks good. Basically the same reasoning as with the cd9660 diff.
ok?
> Index: kern/spec_vnops.c
> ===
> RCS file: /cvs/src/sys/kern/spec_vnops.c,v
> retrieving revision 1.84
> diff -u -p -u -r1.84 spec_vnops.c
> --- kern/spec_vnops.c 5 Dec 2015 10:11:53 - 1.84
> +++ kern/spec_vnops.c 13 Jan 2016 19:12:40 -
> @@ -202,7 +202,8 @@ spec_read(void *v)
> daddr_t bn, nextbn, bscale;
> int bsize;
> struct partinfo dpart;
> - int n, on, majordev;
> + size_t n;
> + int on, majordev;
> int (*ioctl)(dev_t, u_long, caddr_t, int, struct proc *);
> int error = 0;
>
> @@ -243,7 +244,7 @@ spec_read(void *v)
> do {
> bn = btodb(uio->uio_offset) & ~(bscale - 1);
> on = uio->uio_offset % bsize;
> - n = min((bsize - on), uio->uio_resid);
> + n = ulmin((bsize - on), uio->uio_resid);
> if (vp->v_lastr + bscale == bn) {
> nextbn = bn + bscale;
> error = breadn(vp, bn, bsize, &nextbn, &bsize,
> @@ -251,12 +252,12 @@ spec_read(void *v)
> } else
> error = bread(vp, bn, bsize, &bp);
> vp->v_lastr = bn;
> - n = min(n, bsize - bp->b_resid);
> + n = ulmin(n, bsize - bp->b_resid);
> if (error) {
> brelse(bp);
> return (error);
> }
> - error = uiomovei((char *)bp->b_data + on, n, uio);
> + error = uiomove((char *)bp->b_data + on, n, uio);
> brelse(bp);
> } while (error == 0 && uio->uio_resid > 0 && n != 0);
> return (error);
> @@ -290,7 +291,8 @@ spec_write(void *v)
> daddr_t bn, bscale;
> int bsize;
> struct partinfo dpart;
> - int n, on, majordev;
> + size_t n;
> + int on, majordev;
> int (*ioctl)(dev_t, u_long, caddr_t, int, struct proc *);
> int error = 0;
>
> @@ -331,14 +333,14 @@ spec_write(void *v)
> do {
> bn = btodb(uio->uio_offset) & ~(bscale - 1);
> on = uio->uio_offset % bsize;
> - n = min((bsize - on), uio->uio_resid);
> + n = ulmin((bsize - on), uio->uio_resid);
> error = bread(vp, bn, bsize, &bp);
> - n = min(n, bsize - bp->b_resid);
> + n = ulmin(n, bsize - bp->b_resid);
> if (error) {
> brelse(bp);
> return (error);
> }
> - error = uiomovei((char *)bp->b_data + on, n, uio);
> + error = uiomove((char *)bp->b_data + on, n, uio);
> if (n + on == bsize)
> bawrite(bp);
> else
>
> cheers,
> natano
>
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 06:05:31PM +0100, Alexandre Ratchov wrote: > On Mon, Jan 18, 2016 at 01:35:46PM +0100, Sebastien Marie wrote: > > On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote: > > > > I am unsure about returning 0 for something we know is wrong to do. > > heh, it's not wrong; disconnecting audio devices is a legitimate > use of them ;) I didn't want to said that the operation in a whole is wrong, but that making pledge_ioctl() returning 0 would be wrong as we know the operation shouldn't be allowed in normal case. > > Isn't possible to return an error ? As example, calling > > ioctl(TIOCGWINSZ) on no-tty device return ENOTTY. > > > > Agreed, and it's same for chardevs: the ioctl syscall returns > uncondionnaly ENOTTY on disconnected devices: sys_ioctl() calls > vn_ioctl() which returns ENOTTY if vp->v_type == VBAD. hum ? I dunno if I was clear. Returning ENOTTY was an example. Does disconnecting audio devices makes ioctl(2) return ENOTTY ? I doubt. The return code should be the one that normal code path would return. > > There is a difference between: > > - return 0/* let deeper processing happen */ > > - return Exxx /* early return an error */ > > - return pledge_fail()/* kill the process */ > > > > I think the good approch would be to return an error. > > > > You're suggesting to quickly return ENOTTY in pledge_ioctl(), to > avoid going through the regular code-path that returns ENOTTY > anyway? Modulo the ENOTTY error code (see previous comment), yes the purpose is to early return from pledge_ioctl(). pledge(2) permits to expose only a portion of deeper kernel code for a set of defined operations. -- Sebastien Marie
Re: cd9660 uiomove() conversion
Diff reads good. ok?
Some thoughts (mostly for myself) inline.
Martin Natano wrote:
> Below the uiomove conversion for isofs/cd9660/cd9660_vnops.c.
>
> cheers,
> natano
>
> Index: isofs/cd9660/cd9660_vnops.c
> ===
> RCS file: /cvs/src/sys/isofs/cd9660/cd9660_vnops.c,v
> retrieving revision 1.73
> diff -u -p -r1.73 cd9660_vnops.c
> --- isofs/cd9660/cd9660_vnops.c 11 Dec 2015 11:25:55 - 1.73
> +++ isofs/cd9660/cd9660_vnops.c 1 Jan 2016 17:45:50 -
> @@ -227,7 +227,8 @@ cd9660_read(void *v)
> daddr_t lbn, rablock;
> off_t diff;
> int error = 0;
> - long size, n, on;
> + long size, on;
> + size_t n;
>
> if (uio->uio_resid == 0)
> return (0);
> @@ -240,8 +241,7 @@ cd9660_read(void *v)
>
> lbn = lblkno(imp, uio->uio_offset);
> on = blkoff(imp, uio->uio_offset);
> - n = min((u_int)(imp->logical_block_size - on),
> - uio->uio_resid);
> + n = ulmin(imp->logical_block_size - on, uio->uio_resid);
The subtraction can't overflow, because blkoff is basically a
uio->uio_offset % imp->logical_block_size. ulmin() protects against
truncation. The result is always positive and making n size_t is
straightforward.
> diff = (off_t)ip->i_size - uio->uio_offset;
> if (diff <= 0)
> return (0);
> @@ -270,13 +270,13 @@ cd9660_read(void *v)
> } else
> error = bread(vp, lbn, size, &bp);
> ci->ci_lastr = lbn;
> - n = min(n, size - bp->b_resid);
> + n = ulmin(n, size - bp->b_resid);
bp->b_resid is supposed to be <= size because it's the
remaining I/O to get the whole buf filled with size bytes.
So bp->b_resid should be initialized with size initially
and then only decrease.
> if (error) {
> brelse(bp);
> return (error);
> }
>
> - error = uiomovei(bp->b_data + on, (int)n, uio);
> + error = uiomove(bp->b_data + on, n, uio);
Straightforward with n being a size_t.
> brelse(bp);
> } while (error == 0 && uio->uio_resid > 0 && n != 0);
> @@ -344,7 +344,7 @@ iso_uiodir(idp,dp,off)
> }
>
> dp->d_off = off;
> - if ((error = uiomovei((caddr_t)dp, dp->d_reclen, idp->uio)) != 0)
> + if ((error = uiomove(dp, dp->d_reclen, idp->uio)) != 0)
Straightforward. dp->d_reclen is unsigned.
> return (error);
> idp->uio_off = off;
> return (0);
> @@ -657,7 +657,7 @@ cd9660_readlink(void *v)
>*/
> if (uio->uio_segflg != UIO_SYSSPACE ||
> uio->uio_iov->iov_len < MAXPATHLEN) {
> - error = uiomovei(symname, symlen, uio);
> + error = uiomove(symname, symlen, uio);
Straightforward, symlen is unsigned. Also did some checking that computation
of symlen does not wrap around also.
> pool_put(&namei_pool, symname);
> return (error);
> }
>
Re: pledge: audio ioctls on disconnected devices
> On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote: > > > > Unfortunately, if pledge is used, pledge_ioctl() checks if the > > vnode type is VCHR and the process ends up killed. > > > > The diff below fixes this by accepting the audio ioctls if the > > vnode type is VBAD. > > > > I am unsure about returning 0 for something we know is wrong to do. > > Isn't possible to return an error ? As example, calling > ioctl(TIOCGWINSZ) on no-tty device return ENOTTY. > > There is a difference between: > - return 0 /* let deeper processing happen */ > - return Exxx /* early return an error */ > - return pledge_fail() /* kill the process */ > > I think the good approch would be to return an error. I agree. I think this is one of those special cases.
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 01:35:46PM +0100, Sebastien Marie wrote: > On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote: > > > > Unfortunately, if pledge is used, pledge_ioctl() checks if the > > vnode type is VCHR and the process ends up killed. > > > > The diff below fixes this by accepting the audio ioctls if the > > vnode type is VBAD. > > > > I am unsure about returning 0 for something we know is wrong to do. heh, it's not wrong; disconnecting audio devices is a legitimate use of them ;) > Isn't possible to return an error ? As example, calling > ioctl(TIOCGWINSZ) on no-tty device return ENOTTY. > Agreed, and it's same for chardevs: the ioctl syscall returns uncondionnaly ENOTTY on disconnected devices: sys_ioctl() calls vn_ioctl() which returns ENOTTY if vp->v_type == VBAD. > There is a difference between: > - return 0 /* let deeper processing happen */ > - return Exxx /* early return an error */ > - return pledge_fail() /* kill the process */ > > I think the good approch would be to return an error. > You're suggesting to quickly return ENOTTY in pledge_ioctl(), to avoid going through the regular code-path that returns ENOTTY anyway?
Re: [st...@openbsd.org: vlc, ld.so sigsegv/sigbus: _dl_cache_grpsym_list]
On 2016/01/16 15:10, Philip Guenther wrote:
> On Sun, 27 Dec 2015, Stuart Henderson wrote:
> > Widening the audience to tech in case anyone with an idea missed it on
> > ports: it seems some people are having a lot more trouble than just
> > needing to restart build 2 or 3 times.
> ...
> > To replicate
> >
> > cd /usr/ports/x11/vlc
> > make fake
> > cd /usr/ports/pobj/vlc-2.2.1/vlc-2.2.1
> > PATH="/usr/ports/pobj/vlc-2.2.1/fake-amd64/usr/local/bin:$PATH"
> > LD_LIBRARY_PATH="/usr/ports/pobj/vlc-2.2.1/fake-amd64/usr/local/lib:$LD_LIBRARY_PATH"
> > /usr/ports/pobj/vlc-2.2.1/fake-amd64/usr/local/lib/vlc/vlc-cache-gen -f
> > /usr/ports/pobj/vlc-2.2.1/fake-amd64/usr/local/lib/vlc/plugins
> >
> > repeat the last step until it crashes, e.g.
> >
> > /usr/ports/pobj/vlc-2.2.1/vlc-2.2.1/bin/.libs/vlc-cache-gen:/usr/local/lib/libebml.so.3.0:
> > undefined symbol '_ZNSs4_Rep10_M_destroyERKSaIcE'
> > lazy binding failed!
> > Segmentation fault (core dumped)
> >
> > or
> >
> > Bus error (core dumped)
>
> Okay.
Thanks for looking at this which is obviously a tricky area.
> The problem is that vlc-cache-gen dlopen()s a plugin that has a dependency
> (libgio) which is marked nodelete. When the plugin is dlclose()d, that
> dependency is correctly kept around...but other parts of the load group
> *are* unmapped and their elf_object_t structures freed.
>
> You Can't Do That: the objects that make up a load group must be deleted
> all at once or not at all, as they may have resolved relocations to each
> other and there are certainly pointers between their elf_object_t
> structures via the grpref_list, grpsym_list, and load_object members.
>
> So, once a nodelete object is brought in, the entire load group needs to
> be locked in. The diff below does this by changing the nodelete bits to
> add an "open" reference on the root of the load group that pulled in the
> nodelete object instead of a "child" reference on the nodelete itself.
> (The type of reference was always wrong and may have permitted nodelete
> modules being deleted even in simpler cases.)
This makes complete sense.
> Ports question: does libgio still need to be marked nodelete, or was that
> just from when pthread_atfork() handlers weren't unregistered on
> dlclose()?
I'm unsure about libgio, but at least gobject (which is another
implicated library) apparently does: "Since the type system does not
support reloading its data and assumes that libgobject remains loaded
for the lifetime of the process, we should link libgobject with a flag
indicating that it can't be unloaded."
https://mail.gnome.org/archives/commits-list/2014-April/msg02316.html
https://bugzilla.gnome.org/show_bug.cgi?id=707298
> Ok?
Yes, OK.
There's a small side-effect: with LD_DEBUG, now only the first object
from the load group is reported as being 'nodelete'.
$ LD_DEBUG=1 /usr/local/lib/vlc/vlc-cache-gen -f . 2>&1 | grep nodel | uniq
objname /usr/lib/libpthread.so.20.1 is nodelete
objname /usr/local/lib/libgthread-2.0.so.4200.2 is nodelete
I don't know if that's considered important, but if it's desirable to
keep it then this diff relative to yours would do so:
$ LD_DEBUG=1 /usr/local/lib/vlc/vlc-cache-gen -f . 2>&1 | grep nodel | uniq
objname /usr/lib/libpthread.so.20.1 is nodelete
objname /usr/local/lib/libgthread-2.0.so.4200.2 is nodelete
objname /usr/local/lib/libgmodule-2.0.so.4200.2 is nodelete
objname /usr/local/lib/libgio-2.0.so.4200.2 is nodelete
objname /usr/local/lib/libgobject-2.0.so.4200.2 is nodelete
objname /usr/local/lib/libglib-2.0.so.4200.2 is nodelete
--- resolve.c, Mon Jan 18 12:38:27 2016
+++ resolve.c Mon Jan 18 12:47:03 2016
@@ -59,8 +59,12 @@ _dl_add_object(elf_object_t *object)
* in needs to be kept around forever, so add a reference there.
*/
if (object->obj_flags & DF_1_NODELETE &&
- (object->load_object->status & STAT_NODELETE) == 0) {
+ (object->status & STAT_NODELETE) == 0) {
+ object->status |= STAT_NODELETE;
DL_DEB(("objname %s is nodelete\n", object->load_name));
+ }
+ if (object->obj_flags & DF_1_NODELETE &&
+ (object->load_object->status & STAT_NODELETE) == 0) {
object->load_object->opencount++;
object->load_object->status |= STAT_NODELETE;
}
> Philip Guenther
>
>
> Index: resolve.c
> ===
> RCS file: /data/src/openbsd/src/libexec/ld.so/resolve.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 resolve.c
> --- resolve.c 2 Nov 2015 07:02:53 - 1.69
> +++ resolve.c 16 Jan 2016 23:06:46 -
> @@ -54,12 +54,15 @@ elf_object_t *_dl_loading_object;
> void
> _dl_add_object(elf_object_t *object)
> {
> - /* if a .so is marked nodelete, then add a reference */
> + /*
> + * If a .so is marked nodelete, then the entire load group that it's
> + * in needs to be kept around forever, so add a refere
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote: > > Unfortunately, if pledge is used, pledge_ioctl() checks if the > vnode type is VCHR and the process ends up killed. > > The diff below fixes this by accepting the audio ioctls if the > vnode type is VBAD. > I am unsure about returning 0 for something we know is wrong to do. Isn't possible to return an error ? As example, calling ioctl(TIOCGWINSZ) on no-tty device return ENOTTY. There is a difference between: - return 0/* let deeper processing happen */ - return Exxx /* early return an error */ - return pledge_fail()/* kill the process */ I think the good approch would be to return an error. > Index: kern_pledge.c > === > RCS file: /cvs/src/sys/kern/kern_pledge.c,v > retrieving revision 1.146 > diff -u -p -u -p -r1.146 kern_pledge.c > --- kern_pledge.c 9 Jan 2016 06:13:43 - 1.146 > +++ kern_pledge.c 17 Jan 2016 13:43:37 - > @@ -1205,9 +1205,12 @@ pledge_ioctl(struct proc *p, long com, s > case AUDIO_GETENC: > case AUDIO_SETFD: > case AUDIO_GETPROPS: > - if (fp->f_type == DTYPE_VNODE && > - vp->v_type == VCHR && > + if (fp->f_type != DTYPE_VNODE) > + break; > + if (vp->v_type == VCHR && > cdevsw[major(vp->v_rdev)].d_open == audioopen) > + return (0); > + if (vp->v_type == VBAD) > return (0); > } > #endif /* NAUDIO > 0 */ > > -- Sebastien Marie
Re: pledge: audio ioctls on disconnected devices
On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote:
> If a usb audio device is disconnected, the device vnode is closed
> and replaced by a deadfs vnode, of type VBAD. The sndiod process
> still has a descriptor in use for which any ioctl() fails. It's
> supposed to get the return value, notice the error and close the
> file descriptor.
>
> Unfortunately, if pledge is used, pledge_ioctl() checks if the
> vnode type is VCHR and the process ends up killed.
>
> The diff below fixes this by accepting the audio ioctls if the
> vnode type is VBAD.
>
As usb disks and ttys could be disconnected as well, I'm tempted to
make the check for all ioctls. Opinions?
Index: kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.146
diff -u -p -u -p -r1.146 kern_pledge.c
--- kern_pledge.c 9 Jan 2016 06:13:43 - 1.146
+++ kern_pledge.c 18 Jan 2016 11:52:23 -
@@ -1149,8 +1149,11 @@ pledge_ioctl(struct proc *p, long com, s
}
/* fp != NULL was already checked */
- if (fp->f_type == DTYPE_VNODE)
+ if (fp->f_type == DTYPE_VNODE) {
vp = fp->f_data;
+ if (vp->v_type == VBAD)
+ return (0);
+ }
/*
* Further sets of ioctl become available, but are checked a
pledge: audio ioctls on disconnected devices
If a usb audio device is disconnected, the device vnode is closed and replaced by a deadfs vnode, of type VBAD. The sndiod process still has a descriptor in use for which any ioctl() fails. It's supposed to get the return value, notice the error and close the file descriptor. Unfortunately, if pledge is used, pledge_ioctl() checks if the vnode type is VCHR and the process ends up killed. The diff below fixes this by accepting the audio ioctls if the vnode type is VBAD. OK? Index: kern_pledge.c === RCS file: /cvs/src/sys/kern/kern_pledge.c,v retrieving revision 1.146 diff -u -p -u -p -r1.146 kern_pledge.c --- kern_pledge.c 9 Jan 2016 06:13:43 - 1.146 +++ kern_pledge.c 17 Jan 2016 13:43:37 - @@ -1205,9 +1205,12 @@ pledge_ioctl(struct proc *p, long com, s case AUDIO_GETENC: case AUDIO_SETFD: case AUDIO_GETPROPS: - if (fp->f_type == DTYPE_VNODE && - vp->v_type == VCHR && + if (fp->f_type != DTYPE_VNODE) + break; + if (vp->v_type == VCHR && cdevsw[major(vp->v_rdev)].d_open == audioopen) + return (0); + if (vp->v_type == VBAD) return (0); } #endif /* NAUDIO > 0 */
