Re: Term::Cap full revamp
On Fri, Oct 20, 2023 at 11:52:25AM +0200, Marc Espie wrote:
> I guess i will probably leave it alone after this.
> This does quite a few things compared to my former patches.
>
> - totally get rid of eval, it doen't make sense anymore
> - declare variables before they get used, which tends to
> simplify things.
> - change quaint formatting to something more BSD like
> - update documentation to new style of doing OO
> - use defined logic on entry and such
> - always try to run infocmp as a last resort, even if
> we have a path.
> - run infocmp with the best options we have to get a good termcap
> - use \Q\E, which gets rid of termpat entirely
> - dedup the path along the way: for us, /etc/termcap
> and /usr/share/misc/termcap are the same.
> - redo recursion logic by just recording which term values we
> already saw, the max=32 value overflow was absurd, proper parsing
> yields roughly 10 or so tc redirections for xterm, not >32.
I eventually got an occation to test it. Except for the extra debug
print that sneaked. it works well for my use case. (pkg_* tools on a
terminal application from ports that ships its own terminfo entry).
ok matthieu@ with the print removed (see inline).
>
> Index: Cap.pm
> ===
> RCS file: /cvs/src/gnu/usr.bin/perl/cpan/Term-Cap/Cap.pm,v
> retrieving revision 1.3
> diff -u -p -r1.3 Cap.pm
> --- Cap.pm18 Oct 2023 01:49:26 - 1.3
> +++ Cap.pm20 Oct 2023 09:47:05 -
> @@ -16,8 +16,8 @@ sub croak
>
> use strict;
>
> +use v5.16;
> use vars qw($VERSION $VMS_TERMCAP);
> -use vars qw($termpat $state $first $entry);
>
> $VERSION = '1.17';
>
> @@ -33,7 +33,7 @@ Term::Cap - Perl termcap interface
> =head1 SYNOPSIS
>
> require Term::Cap;
> -$terminal = Tgetent Term::Cap { TERM => undef, OSPEED => $ospeed };
> +$terminal = Term::Cap->Tgetent({ TERM => undef, OSPEED => $ospeed });
> $terminal->Trequire(qw/ce ku kd/);
> $terminal->Tgoto('cm', $col, $row, $FH);
> $terminal->Tputs('dl', $count, $FH);
> @@ -75,10 +75,10 @@ if ( $^O eq 'VMS' )
>
> sub termcap_path
> {## private
> -my @termcap_path;
> +my @l;
>
> # $TERMCAP, if it's a filespec
> -push( @termcap_path, $ENV{TERMCAP} )
> +push(@l, $ENV{TERMCAP})
>if (
> ( exists $ENV{TERMCAP} )
> && (
> @@ -87,23 +87,27 @@ sub termcap_path
> : $ENV{TERMCAP} =~ /^\//s
> )
>);
> -if ( ( exists $ENV{TERMPATH} ) && ( $ENV{TERMPATH} ) )
> -{
> -
> +if (exists $ENV{TERMPATH} && $ENV{TERMPATH}) {
> # Add the users $TERMPATH
> -push( @termcap_path, split( /(:|\s+)/, $ENV{TERMPATH} ) );
> -}
> -else
> -{
> -
> +push(@l, split( /(:|\s+)/, $ENV{TERMPATH}));
> +} else {
> # Defaults
> -push( @termcap_path,
> -exists $ENV{'HOME'} ? $ENV{'HOME'} . '/.termcap' : undef,
> -'/etc/termcap', '/usr/share/misc/termcap', );
> + if (exists $ENV{HOME}) {
> + push(@l, $ENV{HOME}.'/.termcap');
> + }
> +push(@l, '/etc/termcap', '/usr/share/misc/termcap', );
> +}
> +my @termcap_path;
> +my $seen = {};
> +for my $i (@l) {
> + next unless -f $i;
> + my $k = join(',', (stat _)[0,1]);
> + next if $seen->{$k};
> + push(@termcap_path, $i);
> + $seen->{$k} = 1;
> }
>
> -# return the list of those termcaps that exist
> -return grep { defined $_ && -f $_ } @termcap_path;
> +return @termcap_path;
> }
>
> =over 4
> @@ -164,195 +168,158 @@ It calls C on failure.
>
> sub Tgetent
> {## public -- static method
> -my $class = shift;
> -my ($self) = @_;
> +my ($class, $self) = @_;
>
> $self = {} unless defined $self;
> bless $self, $class;
>
> -my ( $term, $cap, $search, $field, $max, $tmp_term, $TERMCAP );
> -local ( $termpat, $state, $first, $entry );# used inside eval
> +my ($cap, $field);
> +
> local $_;
>
> # Compute PADDING factor from OSPEED (to be used by Tpad)
> -if ( !$self->{OSPEED} )
> -{
> -if ($^W)
> -{
> +if (!$self->{OSPEED}) {
> +if ($^W) {
> carp "OSPEED was not set, defaulting to 9600";
> }
> $self->{OSPEED} = 9600;
> }
> -if ( $self->{OSPEED} < 16 )
> -{
> -
> +if ($self->{OSPEED} < 16) {
> # delays for old style speeds
> my @pad = (
> 0,200, 133.3, 90.9, 74.3, 66.7, 50, 33.3,
> 16.7, 8.3, 5.5, 4.1, 2,1,.5, .2
> );
> $self->{PADDING} = $pad[ $self->{OSPEED} ];
> -}
> -else
> -{
> +} else {
> $self->{PADDING} = 1 / $self->{OSPEED};
> }
>
> -unless ( $self->{TERM} )
> -{
> - if ( $ENV{TERM} )
> - {
> - $self->{TERM} = $ENV{TERM} ;
> - }
> - else
> - {
> - if ( $^O eq 'MSWin32'
Re: Prevent off-by-one accounting hang in out-of-swap situations
On 22/10/23(Sun) 20:29, Miod Vallat wrote:
> > On 21/10/23(Sat) 14:28, Miod Vallat wrote:
> > > > Stuart, Miod, I wonder if this also help for the off-by-one issue you
> > > > are seeing. It might not.
> > >
> > > It makes the aforementioned issue disappear on the affected machine.
> >
> > Thanks at lot for testing!
>
> Spoke too soon. I have just hit
>
> panic: kernel diagnostic assertion "uvmexp.swpgonly > 0" failed: file
> "/usr/src/sys/uvm/uvm_anon.c", line 121
> Stopped at db_enter+0x8: add #0x4, r14
> TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
> *235984 11904 0 0x14000 0x2000 reaper
> db_enter() at db_enter+0x8
> panic() at panic+0x74
> __assert() at __assert+0x1c
> uvm_anfree_list() at uvm_anfree_list+0x156
> amap_wipeout() at amap_wipeout+0xe6
> uvm_unmap_detach() at uvm_unmap_detach+0x42
> uvm_map_teardown() at uvm_map_teardown+0x104
> uvmspace_free() at uvmspace_free+0x2a
> reaper() at reaper+0x86
> ddb> show uvmexp
> Current UVM status:
> pagesize=4096 (0x1000), pagemask=0xfff, pageshift=12
> 14875 VM pages: 376 active, 2076 inactive, 1 wired, 7418 free (924
> zero)
> min 10% (25) anon, 10% (25) vnode, 5% (12) vtext
> freemin=495, free-target=660, inactive-target=2809, wired-max=4958
> faults=73331603, traps=39755714, intrs=33863551, ctxswitch=11641480
> fpuswitch
> =0
> softint=15742561, syscalls=39755712, kmapent=11
> fault counts:
> noram=1, noanon=0, noamap=0, pgwait=1629, pgrele=0
> ok relocks(total)=1523991(1524022), anget(retries)=23905247(950233),
> amapco
> py=9049749
> neighbor anon/obj pg=12025732/40041442,
> gets(lock/unlock)=12859247/574102
> cases: anon=20680175, anoncow=3225049, obj=11467884, prcopy=1391019,
> przero
> =36545783
> daemon and swap counts:
> woke=6868, revs=6246, scans=3525644, obscans=511526, anscans=2930634
> busy=0, freed=1973275, reactivate=83484, deactivate=3941988
> pageouts=94506, pending=94506, nswget=949421
> nswapdev=1
> swpages=4194415, swpginuse=621, swpgonly=0 paging=0
> kernel pointers:
> objs(kern)=0x8c3ca94c
I wonder if the diff below makes a difference. It's hard to debug and it
might be worth adding a counter for bad swap slots.
Index: uvm/uvm_anon.c
===
RCS file: /cvs/src/sys/uvm/uvm_anon.c,v
retrieving revision 1.56
diff -u -p -r1.56 uvm_anon.c
--- uvm/uvm_anon.c 2 Sep 2023 08:24:40 - 1.56
+++ uvm/uvm_anon.c 22 Oct 2023 21:27:42 -
@@ -116,7 +116,7 @@ uvm_anfree_list(struct vm_anon *anon, st
uvm_unlock_pageq(); /* free the daemon */
}
} else {
- if (anon->an_swslot != 0) {
+ if (anon->an_swslot != 0 && anon->an_swslot != SWSLOT_BAD) {
/* This page is no longer only in swap. */
KASSERT(uvmexp.swpgonly > 0);
atomic_dec_int(&uvmexp.swpgonly);
Re: Prevent off-by-one accounting hang in out-of-swap situations
> On 21/10/23(Sat) 14:28, Miod Vallat wrote: > > > Stuart, Miod, I wonder if this also help for the off-by-one issue you > > > are seeing. It might not. > > > > It makes the aforementioned issue disappear on the affected machine. > > Thanks at lot for testing! Spoke too soon. I have just hit panic: kernel diagnostic assertion "uvmexp.swpgonly > 0" failed: file "/usr/src/sys/uvm/uvm_anon.c", line 121 Stopped at db_enter+0x8: add #0x4, r14 TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *235984 11904 0 0x14000 0x2000 reaper db_enter() at db_enter+0x8 panic() at panic+0x74 __assert() at __assert+0x1c uvm_anfree_list() at uvm_anfree_list+0x156 amap_wipeout() at amap_wipeout+0xe6 uvm_unmap_detach() at uvm_unmap_detach+0x42 uvm_map_teardown() at uvm_map_teardown+0x104 uvmspace_free() at uvmspace_free+0x2a reaper() at reaper+0x86 ddb> show uvmexp Current UVM status: pagesize=4096 (0x1000), pagemask=0xfff, pageshift=12 14875 VM pages: 376 active, 2076 inactive, 1 wired, 7418 free (924 zero) min 10% (25) anon, 10% (25) vnode, 5% (12) vtext freemin=495, free-target=660, inactive-target=2809, wired-max=4958 faults=73331603, traps=39755714, intrs=33863551, ctxswitch=11641480 fpuswitch =0 softint=15742561, syscalls=39755712, kmapent=11 fault counts: noram=1, noanon=0, noamap=0, pgwait=1629, pgrele=0 ok relocks(total)=1523991(1524022), anget(retries)=23905247(950233), amapco py=9049749 neighbor anon/obj pg=12025732/40041442, gets(lock/unlock)=12859247/574102 cases: anon=20680175, anoncow=3225049, obj=11467884, prcopy=1391019, przero =36545783 daemon and swap counts: woke=6868, revs=6246, scans=3525644, obscans=511526, anscans=2930634 busy=0, freed=1973275, reactivate=83484, deactivate=3941988 pageouts=94506, pending=94506, nswget=949421 nswapdev=1 swpages=4194415, swpginuse=621, swpgonly=0 paging=0 kernel pointers: objs(kern)=0x8c3ca94c
Re: nfsd: don't clear SB_NOINTR flag
On Fri, Oct 20, 2023 at 10:51:46PM +0200, Alexander Bluhm wrote:
> On Mon, Oct 16, 2023 at 10:17:50PM +0300, Vitaliy Makkoveev wrote:
> > This socket comes from userland, so this flag is never set. This makes
> > SB_NOINTR flag immutable, because we only set this bit on NFS client
> > socket buffers for all it's lifetime.
> >
> > I want to do this because this flag modifies sblock() behaviour and it's
> > not clean which lock should protect it for standalone sblock().
> >
> > ok?
>
> This code came here:
> https://svnweb.freebsd.org/csrg/sys/nfs/nfs_syscalls.c?revision=41903&view=markup
> "update from Rick Macklem adding TCP support to NFS"
>
> I would guess that this flag must be cleared to allow to kill the
> NFS server if a client does not respond. Otherwise it may hang in
> sbwait() in soreceive() or sosend().
>
> As the flags are never set, your diff does not change behavior.
>
> > I want to completely remove SB_NOINTR flag. Only NFS client sets it, but
> > since the socket never passed to userland, this flag is useless, because
> > we can't send singnal to kernel thread. So for this sblock()/sbwait()
> > sleep is uninterruptible. But I want to not mix NFS server and NFS
> > client diffs.
>
> The NFS client does not run as kernel thread, but as the process
> that does file system access. You can Ctrl-C it if mount uses
> "intr" option. NFSMNT_INT sets some PCATCH and sb_timeo_nsecs, but
> I think signal should also abort sbwait(). That is why NFS client
> sets SB_NOINTR.
>
> As this flag is only set during mount or reconnect. It is a problem
> for MP? Is it modified in a non-initialization path?
>
> bluhm
>
Sorry for non clean explanation.
This time solock() protects `sb_flags', so the following SB_NOINTR flag
check is fine:
sblock(struct socket *so, struct sockbuf *sb, int flags)
{
int error, prio = PSOCK;
soassertlocked(so);
if ((sb->sb_flags & SB_LOCK) == 0) {
sb->sb_flags |= SB_LOCK;
return (0);
}
if ((flags & SBL_WAIT) == 0)
return (EWOULDBLOCK);
if (!(flags & SBL_NOINTR || sb->sb_flags & SB_NOINTR))
prio |= PCATCH;
However, solock() will not protect `sb_flags' for standalone sblock().
It is not yet implemented, so here is the hypothetical sblock(). I
suppose the `sb_lock' rwlock(9) will protect the whole sockbuf data
include `sb_flags'. And there is the problem. As you can see, the
SB_NOINTR check presceeds the `sb_lock' acquisition, because the
rw_enter(9) behaviour depends on this flag.
sblock(struct sockbuf *sb, int flags)
{
int lockflags = RW_WRITE;
if (flags & SBL_NOINTR || sb->sb_flags & SB_NOINTR)
lockflags |= RW_INTR;
if ((flags & SBL_WAIT) == 0)
lockflags |= RW_NOSLEEP;
return rw_enter(&sb->sb_lock, lockflags);
}
I the case, when SB_NOINTR flag is immutable, there is no problem with
this unlocked check. Since this flag is really immutable and it's only
cleanup is the null op, I proposed to remove the flag cleanup.
However, this is not the only solution. This time this flag is not true
for sblock() because you always need to acquire solock() before. The
solock() acquisition is always uninterruptible, so ^C will not work. But
we assume the rwlock(9) acquisition doesn't take significant time,
right?
So, hypothetical sblock() could avoid the SB_NOINTR check. We have
SBL_NOINTR passed with `flags' for the sorflush() cases. This flag is
significant for sbwait(), but with the socket buffers standalone
locking, obviously sblock() should be taken before sbwait(), so the
`sb_flags' check will be protected:
sblock(struct sockbuf *sb, int flags)
{
int lockflags = RW_WRITE;
if (flags & SBL_NOINTR)
lockflags |= RW_INTR;
if ((flags & SBL_WAIT) == 0)
lockflags |= RW_NOSLEEP;
return rw_enter(&sb->sb_lock, lockflags);
}
sbwait(struct sockbuf *sb)
{
int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
sbassertlocked(sb);
return rwsleep_nsec(&sb->sb_cc, &sb->sb_lock, prio, "sbwait",
sb->sb_timeo_nsecs);
}
Of course this is hypothetical. Also, SB_NOINTR flag has sense for TCP
and UDP sockets only. I don't think they will be the first sockets with
standalone locking ability for buffers. The unlocked SB_NOINTR flag
check is fine for all other sockets, so it could be left as is for now.
Re: relayd does not delete control socket on shutdown
Otto Moerbeek wrote: > On Sat, Oct 21, 2023 at 10:40:45PM +0300, Kapetanakis Giannis wrote: > > > On 21/10/2023 20:39, Florian Obser wrote: > > > Which was 8 years ago. I don't understand why you see a change in 7.4. > > > > > > Anyway, we decided to not clean up control sockets in any of our > > > privsep daemons because leaving them behind does not cause any issues. > > > > I just noticed it today when I tried to use the socket in a script and > > noticed that it stayed there even after shutdown and though it was after 7.4 > > but I was wrong about that. > > > > Your commit made it that clear. > > > > Agree it's not a big case if it stays there. > > > > Would the unlink succeed if the socket was owned by _relayd? > > > > G > > Unlinking somthing requires write permissions to the directory it is > in. Which means an attacker who gains control, but otherwise can't do a bunch of other things becuase of the privsep design -- could still fill the directory and filesystem. So a few years ago we asked ourselves -- is the tradeoff worth it?
Re: relayd does not delete control socket on shutdown
On Sat, Oct 21, 2023 at 10:40:45PM +0300, Kapetanakis Giannis wrote: > On 21/10/2023 20:39, Florian Obser wrote: > > Which was 8 years ago. I don't understand why you see a change in 7.4. > > > > Anyway, we decided to not clean up control sockets in any of our > > privsep daemons because leaving them behind does not cause any issues. > > I just noticed it today when I tried to use the socket in a script and > noticed that it stayed there even after shutdown and though it was after 7.4 > but I was wrong about that. > > Your commit made it that clear. > > Agree it's not a big case if it stays there. > > Would the unlink succeed if the socket was owned by _relayd? > > G Unlinking somthing requires write permissions to the directory it is in. -Otto
Re: malloc: more info in error message for write-after-free with option D
Hi,
I wanted an extension to malloc() that would report the caller of all
memory leaks. It works fine for me!
ok asou@
--
ASOU Masato
From: Otto Moerbeek
Date: Tue, 10 Oct 2023 12:39:00 +0200
> Hi,
>
> This diff adds better error reporting for write-after-free or the more
> general write of free memory if malloc option D is active. Knowing the
> place where allocations were done often helps to find out where the
> overwrite happened.
>
> If option D is active malloc now saves caller info in a separate page
> instead of only doing that for chunk index 0. It also reports about
> the preceding chunk if applicable. A report looks like this:
>
> X(12489) in calloc(): write to free chunk 0x851ec6066d0[0..7]@16
> allocated at /usr/X11R6/lib/modules/dri/radeonsi_dri.so 0x88c949
> (preceding chunk 0x851ec6066c0 allocated at /usr/X11R6/bin/X 0x177374 (now
> free))
>
> You can use addr2line -e to get the file and linenumber the allocation
> was done (if the object was compiled with debug info).
>
> If D is not used only the first part is printed, as no caller info is
> saved. So no extra overhead if D is not useed.
>
> Also: the leak report now do not contain unknown locations anymore, as
> each allocation gets its caller recorded if D is active.
>
> -Otto
>
> Index: stdlib/malloc.3
> ===
> RCS file: /home/cvs/src/lib/libc/stdlib/malloc.3,v
> retrieving revision 1.137
> diff -u -p -r1.137 malloc.3
> --- stdlib/malloc.3 1 Jul 2023 18:35:14 - 1.137
> +++ stdlib/malloc.3 10 Oct 2023 10:23:19 -
> @@ -307,7 +307,7 @@ These malloc options imply
> .Cm D .
> .It Cm F
> .Dq Freecheck .
> -Enable more extensive double free and use after free detection.
> +Enable more extensive double free and write after free detection.
> All chunks in the delayed free list will be checked for double frees and
> write after frees.
> Unused pages on the freelist are read and write protected to
> @@ -641,18 +641,34 @@ or
> reallocate an unallocated pointer was made.
> .It Dq double free
> There was an attempt to free an allocation that had already been freed.
> -.It Dq write after free
> -An allocation has been modified after it was freed.
> +.It Dq write of free mem Va address Ns [ Va start Ns .. Ns Va end Ns ]@ Ns
> Va size
> +An allocation has been modified after it was freed,
> +or a chunk that was never allocated was written to.
> +The
> +.Va range
> +at which corruption was detected is printed between [ and ].
> +.Pp
> +Enabling option
> +.Cm D
> +allows malloc to print information about where the allocation
> +was done.
> .It Dq modified chunk-pointer
> The pointer passed to
> .Fn free
> or a reallocation function has been modified.
> -.It Dq canary corrupted address offset@length
> -A byte after the requested size has been overwritten,
> +.It Dq canary corrupted Va address Ns [ Va offset Ns ]@ Ns Va length Ns / Ns
> Va size
> +A byte after the requested
> +.Va length has been overwritten,
> indicating a heap overflow.
> -The offset at which corruption was detected is printed before the @,
> -and the requested length of the allocation after the @.
> -.It Dq recorded size oldsize inconsistent with size
> +The
> +.Va offset
> +at which corruption was detected is printed between [ and ],
> +the requested
> +.Va length
> +of the allocation is printed before the / and the
> +.Va size
> +of the allocation after the /.
> +.It Dq recorded size Va oldsize No inconsistent with Va size
> .Fn recallocarray
> or
> .Fn freezero
> @@ -676,7 +692,7 @@ functions nor utilize any other function
> (e.g.,
> .Xr stdio 3
> routines).
> -.It Dq unknown char in MALLOC_OPTIONS
> +.It Dq unknown char in Ev MALLOC_OPTIONS
> We found something we didn't understand.
> .It any other error
> .Fn malloc
> Index: stdlib/malloc.c
> ===
> RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.290
> diff -u -p -r1.290 malloc.c
> --- stdlib/malloc.c 9 Sep 2023 06:52:40 - 1.290
> +++ stdlib/malloc.c 10 Oct 2023 10:23:19 -
> @@ -112,7 +112,7 @@ struct region_info {
> void *p;/* page; low bits used to mark chunks */
> uintptr_t size; /* size for pages, or chunk_info pointer */
> #ifdef MALLOC_STATS
> - void *f;/* where allocated from */
> + void **f; /* where allocated from */
> #endif
> };
>
> @@ -146,7 +146,7 @@ struct dir_info {
> size_t regions_total; /* number of region slots */
> size_t regions_free;/* number of free slots */
> size_t rbytesused; /* random bytes used */
> - char *func; /* current function */
> + const char *func; /* current function */
> int malloc_junk;/* junk fill? */
> int mmap_flag; /* extra flag for mmap */
>
