Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Kyle Evans

On 5/18/24 23:39, Pedro Giffuni wrote:
FWIW .. and let me be clear I haven't worked on this in ages and I am 
not planning to retake this either...


clang just couldn't do the static  fortify_source checks  due to the way 
llvm uses an intermediate representation; the size just couldn't be 
handled in the preprocessor. Google did spend some time adding extra 
attributes to clang to improve the debugging and you can see that 
implemented in bionic libc but that was it. musl didn't even try.




Admittedly, I have no idea what you're talking about here; none of this 
implementation requires any knowledge of anything at preproc time. 
__builtin_object_size() does the right thing, and the typically 
performance critical string/memory ops use __builtin___foo_chk() that do 
successfully get optimized away in the common case to the underlying 
foo() call.  This all works very well with clang, I haven't tested it 
under GCC but, as you've noted, would assume that it works at least as well.


fortify_source does replace some key libc functions with memory checking 
alternatives and that turns out to be annoying when debugging. In a way 
it breaks that principle C programmers once had, where developers are 
expected to know what they are doing, and if the error is caught at 
runtime by the stack protector anyways it ends up being redundant.

> One more thing about the static checks. Most of the linux distributions
out there indeed have built their software packages with GCC and 
fortify_source >=2. As a consequence, when we ran an exp-run on the 
ports tree (with GCC), fortify_source didn't find anything: it was 
basically a waste of time.


Another reason for not setting it by default is performance. And here I 
answer Shawn's comment on why not enable stack-protector-all and 
safestack and fortify_source at the same time: running unnecessary 
checks over and over again wastes energy and can have some performance 
hit. The later may seem negligible in modern processors, but why do them 
if they bring no benefit? (No need to answer ... just left as food for 
thought)


Pedro.

On Saturday, May 18, 2024 at 09:08:52 PM GMT-5, Kyle Evans 
 wrote:





On 5/18/24 20:09, Pedro Giffuni wrote:
 > (sorry for top posting .. my mailer just sucks)
 > Hi;
 >
 > I used to like the limited static checking FORTIFY_SOURCE provides and
 > when I ran it over FreeBSD it did find a couple of minor issues. It only
 > works for GCC though.
 >

I don't think this is particularly true anymore; I haven't found a case
yet where __builtin_object_size(3) doesn't give me the correct size
while GCC did.  I'd welcome counter-examples here, though -- we have
funding to both finish the project (widen the _FORTIFY_SOURCE net to
more of libc/libsys) and add tests to demonstrate that it's both
functional and correct.  It would be useful to also document
deficiencies in the tests.

 > I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD
 > had the least intrusive implementation the last time I checked but I
 > would certainly request it should never be activated by default,
 > specially with clang. The GCC version has seen more development on glibc
 > but I still think its a dead end.
 >

I don't see a compelling reason to avoid enabling it by default; see
above, the functionality that we need in clang appears to be just fine
(and, iirc, was also fine when I checked at the beginning of working on
this in 2021) and it provides useful

 > What I would like to see working on FreeBSD is Safestack as a
 > replacement for the stack protector, which we were so very slow to adopt
 > even when it was originally developed in FreeBSD. I think other projects
 > based on FreeBSD (Chimera and hardenedBSD) have been using it but I
 > don't know the details.
 >

No comment there, though I think Shawn Webb / HardenedBSD had been
playing around with SafeStack (and might have enabled it? I haven't
actually looked in a while now).

 > This is just all my $0.02
 >
 > Pedro.

Thanks,

Kyle Evans

 >
 > On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans
 > mailto:kaev...@fastmail.com>> wrote:
 >
 >
 >
 >
 > On May 18, 2024 13:42, Pedro Giffuni > wrote:

 >
 >    Oh no .. please not...
 >
 >    We went into that in a GSoC:
 >
 > 
https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions 
 >

 >
 >
 >    Ultimately it proved to be useless since stack-protector-strong.
 >
 >
 > Respectfully, I disagree with your conclusion here:
 >
 > 1.) _FORTIFY_SOURCE provides more granular detection of overflow; I
 > don't have to overflow all the way into the canary at the end of the
 > frame to be detected, so my minor bug now can be caught before something
 > causes the stack frame to be rearranged and 

Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Jörg Sonnenberger
On Sunday, May 19, 2024 6:39:59 AM GMT+2 Pedro Giffuni wrote:
>  FWIW .. and let me be clear I haven't worked on this in ages and I am not 
> planning to retake this either...
> clang just couldn't do the static  fortify_source checks  due to the way llvm 
> uses an intermediate representation; the size just couldn't be handled in the 
> preprocessor.

That hasn't been true for a long time. __builtin_object_size is only
folded in the frontend if it absolutely has to because the context
requires an Integer Constant Expression, otherwise it is folded only
if the answer is definite. In all other cases it is left for the IR passes to
lower.

> fortify_source does replace some key libc functions with memory checking 
> alternatives and that turns out to be annoying when debugging.

Since the wrapper functions are always_inline, they are pretty much invisible 
to debuggers.

Joerg


Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Shawn Webb
On Sat, May 18, 2024 at 09:08:48PM -0500, Kyle Evans wrote:
> 
> 
> On 5/18/24 20:09, Pedro Giffuni wrote:
> > (sorry for top posting .. my mailer just sucks)
> > Hi;
> > 
> > I used to like the limited static checking FORTIFY_SOURCE provides and
> > when I ran it over FreeBSD it did find a couple of minor issues. It only
> > works for GCC though.
> > 
> 
> I don't think this is particularly true anymore; I haven't found a case yet
> where __builtin_object_size(3) doesn't give me the correct size while GCC
> did.  I'd welcome counter-examples here, though -- we have funding to both
> finish the project (widen the _FORTIFY_SOURCE net to more of libc/libsys)
> and add tests to demonstrate that it's both functional and correct.  It
> would be useful to also document deficiencies in the tests.
> 
> > I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD
> > had the least intrusive implementation the last time I checked but I
> > would certainly request it should never be activated by default,
> > specially with clang. The GCC version has seen more development on glibc
> > but I still think its a dead end.
> > 
> 
> I don't see a compelling reason to avoid enabling it by default; see above,
> the functionality that we need in clang appears to be just fine (and, iirc,
> was also fine when I checked at the beginning of working on this in 2021)
> and it provides useful
> 
> > What I would like to see working on FreeBSD is Safestack as a
> > replacement for the stack protector, which we were so very slow to adopt
> > even when it was originally developed in FreeBSD. I think other projects
> > based on FreeBSD (Chimera and hardenedBSD) have been using it but I
> > don't know the details.
> > 
> 
> No comment there, though I think Shawn Webb / HardenedBSD had been playing
> around with SafeStack (and might have enabled it? I haven't actually looked
> in a while now).
 
HardenedBSD has enabled SafeStack for userland applications and base
and a few ports. HardenedBSD uses -fstack-protector-all. I don't see
_FORTIFY_SOURCE, SafeStack, and SSP as mutually exclusive. In fact, I
view all three as complementary.

_FORTIFY_SOURCE can have a much wider reach than SafeStack at the
moment. SafeStack cannot be applied to shared objects, only
dynamically-loaded executables (ELF ET_DYN and ET_EXEC). SafeStack
relies on both ASLR and W^X for efficacy. SafeStack cannot be used
with setjmp/longjmp.

I would like to see SafeStack reach completion and have made attempts
in the past to help push the needle in that direction. We need
explicit support in the RTLD and libc in order to apply it to
libraries. Additionally, we would like to apply it to
statically-linked binaries.

Thanks,

-- 
Shawn Webb
Cofounder / Security Engineer
HardenedBSD

Tor-ified Signal: +1 303-901-1600 / shawn_webb_opsec.50
https://git.hardenedbsd.org/hardenedbsd/pubkeys/-/raw/master/Shawn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc


signature.asc
Description: PGP signature


Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Kyle Evans




On 5/18/24 20:09, Pedro Giffuni wrote:

(sorry for top posting .. my mailer just sucks)
Hi;

I used to like the limited static checking FORTIFY_SOURCE provides and 
when I ran it over FreeBSD it did find a couple of minor issues. It only 
works for GCC though.




I don't think this is particularly true anymore; I haven't found a case 
yet where __builtin_object_size(3) doesn't give me the correct size 
while GCC did.  I'd welcome counter-examples here, though -- we have 
funding to both finish the project (widen the _FORTIFY_SOURCE net to 
more of libc/libsys) and add tests to demonstrate that it's both 
functional and correct.  It would be useful to also document 
deficiencies in the tests.


I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD 
had the least intrusive implementation the last time I checked but I 
would certainly request it should never be activated by default, 
specially with clang. The GCC version has seen more development on glibc 
but I still think its a dead end.




I don't see a compelling reason to avoid enabling it by default; see 
above, the functionality that we need in clang appears to be just fine 
(and, iirc, was also fine when I checked at the beginning of working on 
this in 2021) and it provides useful


What I would like to see working on FreeBSD is Safestack as a 
replacement for the stack protector, which we were so very slow to adopt 
even when it was originally developed in FreeBSD. I think other projects 
based on FreeBSD (Chimera and hardenedBSD) have been using it but I 
don't know the details.




No comment there, though I think Shawn Webb / HardenedBSD had been 
playing around with SafeStack (and might have enabled it? I haven't 
actually looked in a while now).



This is just all my $0.02

Pedro.


Thanks,

Kyle Evans



On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans 
 wrote:





On May 18, 2024 13:42, Pedro Giffuni  wrote:

Oh no .. please not...

We went into that in a GSoC:

https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions 



Ultimately it proved to be useless since stack-protector-strong.


Respectfully, I disagree with your conclusion here:

1.) _FORTIFY_SOURCE provides more granular detection of overflow; I 
don't have to overflow all the way into the canary at the end of the 
frame to be detected, so my minor bug now can be caught before something 
causes the stack frame to be rearranged and turn it into a security 
issue later


2.) __builtin_object_size doesn't work on heap objects, but it actually 
can work on subobjects from a heap allocation (e.g., >name), so the 
coverage extends beyond the stack into starting to detect other kinds of 
overflow


While the security value over stack-protector-strong may be marginal (I 
won't debate this specifically), the feature still has value in general.


Thanks,

Kyle Evans

The NetBSD code was not well adapted to clang either.

Ask me more if you really want to dig into it, but we don't want this.

Pedro.


On Monday, May 13, 2024 at 12:24:13 AM GMT-5, Kyle Evans
 wrote:


The branch main has been updated by kevans:

URL:
https://cgit.FreeBSD.org/src/commit/?id=be04fec42638f30f50b5b55fd8e3634c0fb89928 


commit be04fec42638f30f50b5b55fd8e3634c0fb89928
Author:    Kyle Evans mailto:kev...@freebsd.org>>
AuthorDate: 2024-05-13 05:23:49 +
Commit:    Kyle Evans mailto:kev...@freebsd.org>>
CommitDate: 2024-05-13 05:23:49 +

     Import _FORTIFY_SOURCE implementation from NetBSD

     This is a mostly-unmodified copy of the various *_chk
implementations
     and headers from NetBSD, without yet modifying system headers
to start
     actually including them.  A future commit will also apply the
needed
     bits to fix ssp/unistd.h.

     Reviewed by:    imp, pauamma_gundo.com (both previous
versions), kib
     Sponsored by:  Stormshield
     Sponsored by:  Klara, Inc.
     Differential Revision: https://reviews.freebsd.org/D32306

---
etc/mtree/BSD.include.dist        |  2 +
include/Makefile                  |  2 +-
include/ssp/Makefile              |  6 ++
include/ssp/ssp.h                  |  91 ++
include/ssp/stdio.h                |  93 ++
include/ssp/string.h              | 129

include/ssp/strings.h              |  67 +++
include/ssp/unistd.h              |  54 +++
lib/libc/secure/Makefile.inc      |  11 
lib/libc/secure/Symbol.map        |  18 +
lib/libc/secure/fgets_chk.c        |  54 +++
lib/libc/secure/gets_chk.c        |  74 

Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Pedro Giffuni
 (sorry for top posting .. my mailer just sucks)Hi;
I used to like the limited static checking FORTIFY_SOURCE provides and when I 
ran it over FreeBSD it did find a couple of minor issues. It only works for GCC 
though.
I guess it doesn't really hurt to have FORTIFY_SOURCE around and NetBSD had the 
least intrusive implementation the last time I checked but I would certainly 
request it should never be activated by default, specially with clang. The GCC 
version has seen more development on glibc but I still think its a dead end.
What I would like to see working on FreeBSD is Safestack as a replacement for 
the stack protector, which we were so very slow to adopt even when it was 
originally developed in FreeBSD. I think other projects based on FreeBSD 
(Chimera and hardenedBSD) have been using it but I don't know the details.
This is just all my $0.02
Pedro.
On Saturday, May 18, 2024 at 05:54:42 PM GMT-5, Kyle Evans 
 wrote:  
 
 

On May 18, 2024 13:42, Pedro Giffuni  wrote:

 Oh no .. please not...
We went into that in a GSoC:
https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions


Ultimately it proved to be useless since stack-protector-strong.

Respectfully, I disagree with your conclusion here:
1.) _FORTIFY_SOURCE provides more granular detection of overflow; I don't have 
to overflow all the way into the canary at the end of the frame to be detected, 
so my minor bug now can be caught before something causes the stack frame to be 
rearranged and turn it into a security issue later
2.) __builtin_object_size doesn't work on heap objects, but it actually can 
work on subobjects from a heap allocation (e.g., >name), so the coverage 
extends beyond the stack into starting to detect other kinds of overflow 
While the security value over stack-protector-strong may be marginal (I won't 
debate this specifically), the feature still has value in general.
Thanks,
Kyle Evans

The NetBSD code was not well adapted to clang either.
Ask me more if you really want to dig into it, but we don't want this.
Pedro.

On Monday, May 13, 2024 at 12:24:13 AM GMT-5, Kyle Evans 
 wrote:  
 
 The branch main has been updated by kevans:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=be04fec42638f30f50b5b55fd8e3634c0fb89928

commit be04fec42638f30f50b5b55fd8e3634c0fb89928
Author:    Kyle Evans 
AuthorDate: 2024-05-13 05:23:49 +
Commit:    Kyle Evans 
CommitDate: 2024-05-13 05:23:49 +

    Import _FORTIFY_SOURCE implementation from NetBSD
    
    This is a mostly-unmodified copy of the various *_chk implementations
    and headers from NetBSD, without yet modifying system headers to start
    actually including them.  A future commit will also apply the needed
    bits to fix ssp/unistd.h.
    
    Reviewed by:    imp, pauamma_gundo.com (both previous versions), kib
    Sponsored by:  Stormshield
    Sponsored by:  Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D32306
---
 etc/mtree/BSD.include.dist        |  2 +
 include/Makefile                  |  2 +-
 include/ssp/Makefile              |  6 ++
 include/ssp/ssp.h                  |  91 ++
 include/ssp/stdio.h                |  93 ++
 include/ssp/string.h              | 129 
 include/ssp/strings.h              |  67 +++
 include/ssp/unistd.h              |  54 +++
 lib/libc/secure/Makefile.inc      |  11 
 lib/libc/secure/Symbol.map        |  18 +
 lib/libc/secure/fgets_chk.c        |  54 +++
 lib/libc/secure/gets_chk.c        |  74 +
 lib/libc/secure/memcpy_chk.c      |  53 +++
 lib/libc/secure/memmove_chk.c      |  47 +
 lib/libc/secure/memset_chk.c      |  46 +
 lib/libc/secure/snprintf_chk.c    |  56 
 lib/libc/secure/sprintf_chk.c      |  61 +
 lib/libc/secure/ssp_internal.h    |  37 +++
 lib/libc/secure/stpcpy_chk.c      |  55 
 lib/libc/secure/stpncpy_chk.c      |  53 +++
 lib/libc/secure/strcat_chk.c      |  60 +
 lib/libc/secure/strcpy_chk.c      |  54 +++
 lib/libc/secure/strncat_chk.c      |  70 
 lib/libc/secure/strncpy_chk.c      |  53 +++
 lib/libc/secure/vsnprintf_chk.c    |  49 ++
 lib/libc/secure/vsprintf_chk.c    |  58 
 lib/libssp/Makefile                |  20 +-
 lib/libssp/Symbol.map              |  12 ++--
 lib/libssp/Versions.def            |  5 ++
 lib/libssp/__builtin_object_size.3 | 110 +++
 lib/libssp/fortify_stubs.c        | 131 -
 lib/libssp/ssp.3                  | 130 
 32 files changed, 1621 insertions(+), 140 deletions(-)

diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.dist
index a6bd5880bf61..f8c83d6dde7a 100644
--- 

git: e4df9d84b19f - stable/13 - vfs_domount_update(): style, use space instead of tab

2024-05-18 Thread Konstantin Belousov
The branch stable/13 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=e4df9d84b19f2e6d1930abf65ef0679196600114

commit e4df9d84b19f2e6d1930abf65ef0679196600114
Author: Konstantin Belousov 
AuthorDate: 2024-05-16 00:59:21 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:59:13 +

vfs_domount_update(): style, use space instead of tab

(cherry picked from commit 5a061a38cdfa151315051a1ca1400bb32e39cde2)
---
 sys/kern/vfs_mount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c
index 756ac4789c8c..68b22d1a77a8 100644
--- a/sys/kern/vfs_mount.c
+++ b/sys/kern/vfs_mount.c
@@ -1359,7 +1359,7 @@ vfs_domount_update(
 
MNT_ILOCK(mp);
if (error == 0) {
-   mp->mnt_flag &= ~(MNT_UPDATE | MNT_RELOAD | MNT_FORCE |
+   mp->mnt_flag &= ~(MNT_UPDATE | MNT_RELOAD | MNT_FORCE |
MNT_SNAPSHOT);
} else {
/*



git: 7c23465541f3 - stable/13 - devfs_allocv(): style

2024-05-18 Thread Konstantin Belousov
The branch stable/13 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=7c23465541f328acab2192c870be31324ce436d9

commit 7c23465541f328acab2192c870be31324ce436d9
Author: Konstantin Belousov 
AuthorDate: 2024-05-07 12:33:01 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:59:13 +

devfs_allocv(): style

(cherry picked from commit 6d79564fe341c8dbf09405cae1a0a76460aaf8aa)
---
 sys/fs/devfs/devfs_vnops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c
index 919ab2ff4381..535b2c198e64 100644
--- a/sys/fs/devfs/devfs_vnops.c
+++ b/sys/fs/devfs/devfs_vnops.c
@@ -574,8 +574,7 @@ loop:
if (devfs_allocv_drop_refs(0, dmp, de)) {
vput(vp);
return (ENOENT);
-   }
-   else if (VN_IS_DOOMED(vp)) {
+   } else if (VN_IS_DOOMED(vp)) {
mtx_lock(_de_interlock);
if (de->de_vnode == vp) {
de->de_vnode = NULL;



git: 2eeb0e9fc130 - stable/13 - cdev_pager_allocate(): ensure that the cdev_pager_ops ctr is called only once

2024-05-18 Thread Konstantin Belousov
The branch stable/13 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=2eeb0e9fc1306f40ec684af1ea56a8871a9a3684

commit 2eeb0e9fc1306f40ec684af1ea56a8871a9a3684
Author: Konstantin Belousov 
AuthorDate: 2024-05-07 13:23:28 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:59:13 +

cdev_pager_allocate(): ensure that the cdev_pager_ops ctr is called only 
once

PR: 278826

(cherry picked from commit e93404065177d6c909cd64bf7d74fe0d8df35edf)
---
 sys/vm/device_pager.c | 70 +--
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/sys/vm/device_pager.c b/sys/vm/device_pager.c
index 2d8f003c6f92..cb3b0e522d9a 100644
--- a/sys/vm/device_pager.c
+++ b/sys/vm/device_pager.c
@@ -118,8 +118,15 @@ cdev_pager_lookup(void *handle)
 {
vm_object_t object;
 
+again:
mtx_lock(_pager_mtx);
object = vm_pager_object_lookup(_pager_object_list, handle);
+   if (object != NULL && object->un_pager.devp.dev == NULL) {
+   msleep(>un_pager.devp.dev, _pager_mtx,
+   PVM | PDROP, "cdplkp", 0);
+   vm_object_deallocate(object);
+   goto again;
+   }
mtx_unlock(_pager_mtx);
return (object);
 }
@@ -129,9 +136,8 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
 const struct cdev_pager_ops *ops, vm_ooffset_t size, vm_prot_t prot,
 vm_ooffset_t foff, struct ucred *cred)
 {
-   vm_object_t object, object1;
+   vm_object_t object;
vm_pindex_t pindex;
-   u_short color;
 
if (tp != OBJT_DEVICE && tp != OBJT_MGTDEVICE)
return (NULL);
@@ -157,16 +163,16 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
pindex < OFF_TO_IDX(size))
return (NULL);
 
-   if (ops->cdev_pg_ctor(handle, size, prot, foff, cred, ) != 0)
-   return (NULL);
+again:
mtx_lock(_pager_mtx);
 
/*
 * Look up pager, creating as necessary.
 */
-   object1 = NULL;
object = vm_pager_object_lookup(_pager_object_list, handle);
if (object == NULL) {
+   vm_object_t object1;
+
/*
 * Allocate object and associate it with the pager.  Initialize
 * the object's pg_color based upon the physical address of the
@@ -174,15 +180,19 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
 */
mtx_unlock(_pager_mtx);
object1 = vm_object_allocate(tp, pindex);
-   object1->flags |= OBJ_COLORED;
-   object1->pg_color = color;
-   object1->handle = handle;
-   object1->un_pager.devp.ops = ops;
-   object1->un_pager.devp.dev = handle;
-   TAILQ_INIT(>un_pager.devp.devp_pglist);
mtx_lock(_pager_mtx);
object = vm_pager_object_lookup(_pager_object_list, handle);
if (object != NULL) {
+   object1->type = OBJT_DEAD;
+   vm_object_deallocate(object1);
+   object1 = NULL;
+   if (object->un_pager.devp.dev == NULL) {
+   msleep(>un_pager.devp.dev,
+   _pager_mtx, PVM | PDROP, "cdplkp", 0);
+   vm_object_deallocate(object);
+   goto again;
+   }
+
/*
 * We raced with other thread while allocating object.
 */
@@ -194,29 +204,51 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
KASSERT(object->un_pager.devp.ops == ops,
("Inconsistent devops %p %p", object, ops));
} else {
+   u_short color;
+
object = object1;
object1 = NULL;
object->handle = handle;
+   object->un_pager.devp.ops = ops;
+   TAILQ_INIT(>un_pager.devp.devp_pglist);
TAILQ_INSERT_TAIL(_pager_object_list, object,
pager_object_list);
+   mtx_unlock(_pager_mtx);
if (ops->cdev_pg_populate != NULL)
vm_object_set_flag(object, OBJ_POPULATE);
+   if (ops->cdev_pg_ctor(handle, size, prot, foff,
+   cred, ) != 0) {
+   mtx_lock(_pager_mtx);
+   TAILQ_REMOVE(_pager_object_list, object,
+   pager_object_list);
+   wakeup(>un_pager.devp.dev);
+   mtx_unlock(_pager_mtx);
+   object->type = OBJT_DEAD;
+   

git: 936aa8ec7cb5 - stable/14 - vfs_domount_update(): style, use space instead of tab

2024-05-18 Thread Konstantin Belousov
The branch stable/14 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=936aa8ec7cb57a32387e1f6e636d055d3397edcf

commit 936aa8ec7cb57a32387e1f6e636d055d3397edcf
Author: Konstantin Belousov 
AuthorDate: 2024-05-16 00:59:21 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:57:54 +

vfs_domount_update(): style, use space instead of tab

(cherry picked from commit 5a061a38cdfa151315051a1ca1400bb32e39cde2)
---
 sys/kern/vfs_mount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c
index 49d1d21d2d6d..9289c35e4f86 100644
--- a/sys/kern/vfs_mount.c
+++ b/sys/kern/vfs_mount.c
@@ -1518,7 +1518,7 @@ vfs_domount_update(
 
MNT_ILOCK(mp);
if (error == 0) {
-   mp->mnt_flag &= ~(MNT_UPDATE | MNT_RELOAD | MNT_FORCE |
+   mp->mnt_flag &= ~(MNT_UPDATE | MNT_RELOAD | MNT_FORCE |
MNT_SNAPSHOT);
} else {
/*



git: 9ccd5394123b - stable/14 - devfs_allocv(): style

2024-05-18 Thread Konstantin Belousov
The branch stable/14 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=9ccd5394123bf429b09b4404fa6cb8543b617331

commit 9ccd5394123bf429b09b4404fa6cb8543b617331
Author: Konstantin Belousov 
AuthorDate: 2024-05-07 12:33:01 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:57:54 +

devfs_allocv(): style

(cherry picked from commit 6d79564fe341c8dbf09405cae1a0a76460aaf8aa)
---
 sys/fs/devfs/devfs_vnops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c
index cda4afc8c241..9dcf3b235feb 100644
--- a/sys/fs/devfs/devfs_vnops.c
+++ b/sys/fs/devfs/devfs_vnops.c
@@ -557,8 +557,7 @@ loop:
if (devfs_allocv_drop_refs(0, dmp, de)) {
vput(vp);
return (ENOENT);
-   }
-   else if (VN_IS_DOOMED(vp)) {
+   } else if (VN_IS_DOOMED(vp)) {
mtx_lock(_de_interlock);
if (de->de_vnode == vp) {
de->de_vnode = NULL;



git: 4018bcdea8e1 - stable/14 - cdev_pager_allocate(): ensure that the cdev_pager_ops ctr is called only once

2024-05-18 Thread Konstantin Belousov
The branch stable/14 has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=4018bcdea8e1934eedba4b800e6feb2099b1091d

commit 4018bcdea8e1934eedba4b800e6feb2099b1091d
Author: Konstantin Belousov 
AuthorDate: 2024-05-07 13:23:28 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-19 00:57:54 +

cdev_pager_allocate(): ensure that the cdev_pager_ops ctr is called only 
once

PR: 278826

(cherry picked from commit e93404065177d6c909cd64bf7d74fe0d8df35edf)
---
 sys/vm/device_pager.c | 70 +--
 1 file changed, 51 insertions(+), 19 deletions(-)

diff --git a/sys/vm/device_pager.c b/sys/vm/device_pager.c
index 2d8f003c6f92..cb3b0e522d9a 100644
--- a/sys/vm/device_pager.c
+++ b/sys/vm/device_pager.c
@@ -118,8 +118,15 @@ cdev_pager_lookup(void *handle)
 {
vm_object_t object;
 
+again:
mtx_lock(_pager_mtx);
object = vm_pager_object_lookup(_pager_object_list, handle);
+   if (object != NULL && object->un_pager.devp.dev == NULL) {
+   msleep(>un_pager.devp.dev, _pager_mtx,
+   PVM | PDROP, "cdplkp", 0);
+   vm_object_deallocate(object);
+   goto again;
+   }
mtx_unlock(_pager_mtx);
return (object);
 }
@@ -129,9 +136,8 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
 const struct cdev_pager_ops *ops, vm_ooffset_t size, vm_prot_t prot,
 vm_ooffset_t foff, struct ucred *cred)
 {
-   vm_object_t object, object1;
+   vm_object_t object;
vm_pindex_t pindex;
-   u_short color;
 
if (tp != OBJT_DEVICE && tp != OBJT_MGTDEVICE)
return (NULL);
@@ -157,16 +163,16 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
pindex < OFF_TO_IDX(size))
return (NULL);
 
-   if (ops->cdev_pg_ctor(handle, size, prot, foff, cred, ) != 0)
-   return (NULL);
+again:
mtx_lock(_pager_mtx);
 
/*
 * Look up pager, creating as necessary.
 */
-   object1 = NULL;
object = vm_pager_object_lookup(_pager_object_list, handle);
if (object == NULL) {
+   vm_object_t object1;
+
/*
 * Allocate object and associate it with the pager.  Initialize
 * the object's pg_color based upon the physical address of the
@@ -174,15 +180,19 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
 */
mtx_unlock(_pager_mtx);
object1 = vm_object_allocate(tp, pindex);
-   object1->flags |= OBJ_COLORED;
-   object1->pg_color = color;
-   object1->handle = handle;
-   object1->un_pager.devp.ops = ops;
-   object1->un_pager.devp.dev = handle;
-   TAILQ_INIT(>un_pager.devp.devp_pglist);
mtx_lock(_pager_mtx);
object = vm_pager_object_lookup(_pager_object_list, handle);
if (object != NULL) {
+   object1->type = OBJT_DEAD;
+   vm_object_deallocate(object1);
+   object1 = NULL;
+   if (object->un_pager.devp.dev == NULL) {
+   msleep(>un_pager.devp.dev,
+   _pager_mtx, PVM | PDROP, "cdplkp", 0);
+   vm_object_deallocate(object);
+   goto again;
+   }
+
/*
 * We raced with other thread while allocating object.
 */
@@ -194,29 +204,51 @@ cdev_pager_allocate(void *handle, enum obj_type tp,
KASSERT(object->un_pager.devp.ops == ops,
("Inconsistent devops %p %p", object, ops));
} else {
+   u_short color;
+
object = object1;
object1 = NULL;
object->handle = handle;
+   object->un_pager.devp.ops = ops;
+   TAILQ_INIT(>un_pager.devp.devp_pglist);
TAILQ_INSERT_TAIL(_pager_object_list, object,
pager_object_list);
+   mtx_unlock(_pager_mtx);
if (ops->cdev_pg_populate != NULL)
vm_object_set_flag(object, OBJ_POPULATE);
+   if (ops->cdev_pg_ctor(handle, size, prot, foff,
+   cred, ) != 0) {
+   mtx_lock(_pager_mtx);
+   TAILQ_REMOVE(_pager_object_list, object,
+   pager_object_list);
+   wakeup(>un_pager.devp.dev);
+   mtx_unlock(_pager_mtx);
+   object->type = OBJT_DEAD;
+   

Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Kyle Evans
On May 18, 2024 13:42, Pedro Giffuni  wrote:
Oh no .. please not...We went into that in a GSoC:https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensionsUltimately it proved to be useless since stack-protector-strong.Respectfully, I disagree with your conclusion here:1.) _FORTIFY_SOURCE provides more granular detection of overflow; I don't have to overflow all the way into the canary at the end of the frame to be detected, so my minor bug now can be caught before something causes the stack frame to be rearranged and turn it into a security issue later2.) __builtin_object_size doesn't work on heap objects, but it actually can work on subobjects from a heap allocation (e.g., >name), so the coverage extends beyond the stack into starting to detect other kinds of overflow While the security value over stack-protector-strong may be marginal (I won't debate this specifically), the feature still has value in general.Thanks,Kyle EvansThe NetBSD code was not well adapted to clang either.Ask me more if you really want to dig into it, but we don't want this.Pedro.





On Monday, May 13, 2024 at 12:24:13 AM GMT-5, Kyle Evans  wrote:





The branch main has been updated by kevans:URL: https://cgit.FreeBSD.org/src/commit/?id=be04fec42638f30f50b5b55fd8e3634c0fb89928commit be04fec42638f30f50b5b55fd8e3634c0fb89928Author:     Kyle Evans AuthorDate: 2024-05-13 05:23:49 +Commit:     Kyle Evans CommitDate: 2024-05-13 05:23:49 +    Import _FORTIFY_SOURCE implementation from NetBSD        This is a mostly-unmodified copy of the various *_chk implementations    and headers from NetBSD, without yet modifying system headers to start    actually including them.  A future commit will also apply the needed    bits to fix ssp/unistd.h.        Reviewed by:    imp, pauamma_gundo.com (both previous versions), kib    Sponsored by:   Stormshield    Sponsored by:   Klara, Inc.    Differential Revision:  https://reviews.freebsd.org/D32306--- etc/mtree/BSD.include.dist         |   2 + include/Makefile                   |   2 +- include/ssp/Makefile               |   6 ++ include/ssp/ssp.h                  |  91 ++ include/ssp/stdio.h                |  93 ++ include/ssp/string.h               | 129  include/ssp/strings.h              |  67 +++ include/ssp/unistd.h               |  54 +++ lib/libc/secure/Makefile.inc       |  11  lib/libc/secure/Symbol.map         |  18 + lib/libc/secure/fgets_chk.c        |  54 +++ lib/libc/secure/gets_chk.c         |  74 + lib/libc/secure/memcpy_chk.c       |  53 +++ lib/libc/secure/memmove_chk.c      |  47 + lib/libc/secure/memset_chk.c       |  46 + lib/libc/secure/snprintf_chk.c     |  56  lib/libc/secure/sprintf_chk.c      |  61 + lib/libc/secure/ssp_internal.h     |  37 +++ lib/libc/secure/stpcpy_chk.c       |  55  lib/libc/secure/stpncpy_chk.c      |  53 +++ lib/libc/secure/strcat_chk.c       |  60 + lib/libc/secure/strcpy_chk.c       |  54 +++ lib/libc/secure/strncat_chk.c      |  70  lib/libc/secure/strncpy_chk.c      |  53 +++ lib/libc/secure/vsnprintf_chk.c    |  49 ++ lib/libc/secure/vsprintf_chk.c     |  58  lib/libssp/Makefile                |  20 +- lib/libssp/Symbol.map              |  12 ++-- lib/libssp/Versions.def            |   5 ++ lib/libssp/__builtin_object_size.3 | 110 +++ lib/libssp/fortify_stubs.c         | 131 - lib/libssp/ssp.3                   | 130  32 files changed, 1621 insertions(+), 140 deletions(-)diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.distindex a6bd5880bf61..f8c83d6dde7a 100644--- a/etc/mtree/BSD.include.dist+++ b/etc/mtree/BSD.include.dist@@ -372,6 +372,8 @@         mac_veriexec         ..     ..+    ssp+    ..     sys         disk         ..diff --git a/include/Makefile b/include/Makefileindex 19e6beb95203..32774419f162 100644--- a/include/Makefile+++ b/include/Makefile@@ -4,7 +4,7 @@  PACKAGE=clibs CLEANFILES= osreldate.h version-SUBDIR= arpa protocols rpcsvc rpc xlocale+SUBDIR= arpa protocols rpcsvc rpc ssp xlocale .if ${MACHINE_CPUARCH} == "amd64" SUBDIR+=        i386 INCLUDE_SUBDIRS+=    i386diff --git a/include/ssp/Makefile b/include/ssp/Makefilenew file mode 100644index ..dff19f43c920--- /dev/null+++ b/include/ssp/Makefile@@ -0,0 +1,6 @@+# $FreeBSD$++INCS=    ssp.h stdio.h string.h strings.h unistd.h+INCSDIR=    

git: ff4480baf6f1 - main - nfs client comment typo fix

2024-05-18 Thread Konstantin Belousov
The branch main has been updated by kib:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=ff4480baf6f15551aee58707bffc9b15608d89d8

commit ff4480baf6f15551aee58707bffc9b15608d89d8
Author: Konstantin Belousov 
AuthorDate: 2024-05-18 22:49:59 +
Commit: Konstantin Belousov 
CommitDate: 2024-05-18 22:49:59 +

nfs client comment typo fix

Sponsored by:   The FreeBSD Foundation
MFC after:  3 days
---
 sys/fs/nfsclient/nfs_clbio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c
index ed7149c27903..fe2ed0dff0dd 100644
--- a/sys/fs/nfsclient/nfs_clbio.c
+++ b/sys/fs/nfsclient/nfs_clbio.c
@@ -366,7 +366,7 @@ nfs_bioread_check_cons(struct vnode *vp, struct thread *td, 
struct ucred *cred)
bool old_lock;
 
/*
-* Ensure the exclusove access to the node before checking
+* Ensure the exclusive access to the node before checking
 * whether the cache is consistent.
 */
old_lock = ncl_excl_start(vp);



Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Pedro F. Giffuni
 It was just pointed out to me this sounded insulting .. for which I am sorry.
It's been almost 10 years since I last looked at it and it is disappointing to 
see my almost-mistake happening. This was a lot of work by Kyle and I didn't 
mean to be dismissive of it.
So ]I'll just shudup and creep back into my hole, sorry for disturbing the real 
work going on.
Pedro.
On Saturday, May 18, 2024 at 01:42:54 PM GMT-5, Pedro Giffuni 
 wrote:  
 
  Oh no .. please not...
We went into that in a GSoC:
https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions


Ultimately it proved to be useless since stack-protector-strong.
The NetBSD code was not well adapted to clang either.
Ask me more if you really want to dig into it, but we don't want this.
Pedro.

On Monday, May 13, 2024 at 12:24:13 AM GMT-5, Kyle Evans 
 wrote:  
 
 The branch main has been updated by kevans:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=be04fec42638f30f50b5b55fd8e3634c0fb89928

commit be04fec42638f30f50b5b55fd8e3634c0fb89928
Author:    Kyle Evans 
AuthorDate: 2024-05-13 05:23:49 +
Commit:    Kyle Evans 
CommitDate: 2024-05-13 05:23:49 +

    Import _FORTIFY_SOURCE implementation from NetBSD
    
    This is a mostly-unmodified copy of the various *_chk implementations
    and headers from NetBSD, without yet modifying system headers to start
    actually including them.  A future commit will also apply the needed
    bits to fix ssp/unistd.h.
    
    Reviewed by:    imp, pauamma_gundo.com (both previous versions), kib
    Sponsored by:  Stormshield
    Sponsored by:  Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D32306
---
 etc/mtree/BSD.include.dist        |  2 +
 include/Makefile                  |  2 +-
 include/ssp/Makefile              |  6 ++
 include/ssp/ssp.h                  |  91 ++
 include/ssp/stdio.h                |  93 ++
 include/ssp/string.h              | 129 
 include/ssp/strings.h              |  67 +++
 include/ssp/unistd.h              |  54 +++
 lib/libc/secure/Makefile.inc      |  11 
 lib/libc/secure/Symbol.map        |  18 +
 lib/libc/secure/fgets_chk.c        |  54 +++
 lib/libc/secure/gets_chk.c        |  74 +
 lib/libc/secure/memcpy_chk.c      |  53 +++
 lib/libc/secure/memmove_chk.c      |  47 +
 lib/libc/secure/memset_chk.c      |  46 +
 lib/libc/secure/snprintf_chk.c    |  56 
 lib/libc/secure/sprintf_chk.c      |  61 +
 lib/libc/secure/ssp_internal.h    |  37 +++
 lib/libc/secure/stpcpy_chk.c      |  55 
 lib/libc/secure/stpncpy_chk.c      |  53 +++
 lib/libc/secure/strcat_chk.c      |  60 +
 lib/libc/secure/strcpy_chk.c      |  54 +++
 lib/libc/secure/strncat_chk.c      |  70 
 lib/libc/secure/strncpy_chk.c      |  53 +++
 lib/libc/secure/vsnprintf_chk.c    |  49 ++
 lib/libc/secure/vsprintf_chk.c    |  58 
 lib/libssp/Makefile                |  20 +-
 lib/libssp/Symbol.map              |  12 ++--
 lib/libssp/Versions.def            |  5 ++
 lib/libssp/__builtin_object_size.3 | 110 +++
 lib/libssp/fortify_stubs.c        | 131 -
 lib/libssp/ssp.3                  | 130 
 32 files changed, 1621 insertions(+), 140 deletions(-)

diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.dist
index a6bd5880bf61..f8c83d6dde7a 100644
--- a/etc/mtree/BSD.include.dist
+++ b/etc/mtree/BSD.include.dist
@@ -372,6 +372,8 @@
        mac_veriexec
        ..
    ..
+    ssp
+    ..
    sys
        disk
        ..
diff --git a/include/Makefile b/include/Makefile
index 19e6beb95203..32774419f162 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -4,7 +4,7 @@
 
 PACKAGE=clibs
 CLEANFILES= osreldate.h version
-SUBDIR= arpa protocols rpcsvc rpc xlocale
+SUBDIR= arpa protocols rpcsvc rpc ssp xlocale
 .if ${MACHINE_CPUARCH} == "amd64"
 SUBDIR+=        i386
 INCLUDE_SUBDIRS+=    i386
diff --git a/include/ssp/Makefile b/include/ssp/Makefile
new file mode 100644
index ..dff19f43c920
--- /dev/null
+++ b/include/ssp/Makefile
@@ -0,0 +1,6 @@
+# $FreeBSD$
+
+INCS=    ssp.h stdio.h string.h strings.h unistd.h
+INCSDIR=    ${INCLUDEDIR}/ssp
+
+.include 
diff --git a/include/ssp/ssp.h b/include/ssp/ssp.h
new file mode 100644
index ..35a9aeee02df
--- /dev/null
+++ b/include/ssp/ssp.h
@@ -0,0 +1,91 @@
+/*    $NetBSD: ssp.h,v 1.13 2015/09/03 20:43:47 plunky Exp $    */
+
+/*-
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2006, 2011 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Christos Zoulas.
+ *
+ * 

Re: git: 9bfd3b4076a7 - main - Add a build knob for _FORTIFY_SOURCE

2024-05-18 Thread Pedro Giffuni
 Sorry for noticing so late ... I was unaware this was being worked on and  I 
was very busy with since my dad passed away recently.
The static checker component of Fortify source only works well on GCC, for 
clang this lacks the support that was added by Google on Android's libc (which 
is not bery useful either).
We already had some stubs for the ssp functions but we never used them and this 
just adds bloat to our libc.
I suggest reverting. Again sorry.
Pedro.
On Monday, May 13, 2024 at 12:24:16 AM GMT-5, Kyle Evans 
 wrote:  
 
 The branch main has been updated by kevans:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=9bfd3b4076a7b0dfd27ab22318e5113dc84fea28

commit 9bfd3b4076a7b0dfd27ab22318e5113dc84fea28
Author:    Kyle Evans 
AuthorDate: 2024-05-13 05:23:50 +
Commit:    Kyle Evans 
CommitDate: 2024-05-13 05:23:50 +

    Add a build knob for _FORTIFY_SOURCE
    
    In the future, we will Default to _FORTIFY_SOURCE=2 if SSP is enabled,
    otherwise default to _FORTIFY_SOURCE=0.  For now we default it to 0
    unconditionally to ease bisect across older versions without the new
    symbols, and we'll put out a call for testing.
    
    include/*.h include their ssp/*.h equivalents as needed based on the
    knob. Programs and users are allowed to override FORTIFY_SOURCE in their
    Makefiles or src.conf/make.conf to force it off.
    
    Reviewed by:    des, markj
    Relnotes:      yes
    Sponsored by:  Stormshield
    Sponsored by:  Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D32308
---
 include/stdio.h                |  3 ++
 include/string.h                |  3 ++
 include/strings.h              |  3 ++
 include/unistd.h                |  4 +++
 lib/libthr/Makefile            |  3 ++
 libexec/rtld-elf/Makefile      |  4 +++
 share/man/man7/security.7      | 75 +
 share/mk/bsd.sys.mk            |  7 
 tools/build/options/WITHOUT_SSP |  3 ++
 tools/build/options/WITH_SSP    |  3 ++
 10 files changed, 108 insertions(+)

diff --git a/include/stdio.h b/include/stdio.h
index fe7a6f7d6f82..30bc638082d8 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -530,4 +530,7 @@ extern int __isthreaded;
 __END_DECLS
 __NULLABILITY_PRAGMA_POP
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include 
+#endif
 #endif /* !_STDIO_H_ */
diff --git a/include/string.h b/include/string.h
index 597308020cdb..a595f6e3e260 100644
--- a/include/string.h
+++ b/include/string.h
@@ -168,4 +168,7 @@ errno_t memset_s(void *, rsize_t, int, rsize_t);
 #endif /* __EXT1_VISIBLE */
 __END_DECLS
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include 
+#endif
 #endif /* _STRING_H_ */
diff --git a/include/strings.h b/include/strings.h
index fde007186e04..6fe6a09e7dd3 100644
--- a/include/strings.h
+++ b/include/strings.h
@@ -68,4 +68,7 @@ int    strncasecmp(const char *, const char *, size_t) __pure;
 #endif
 __END_DECLS
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include 
+#endif
 #endif /* _STRINGS_H_ */
diff --git a/include/unistd.h b/include/unistd.h
index e4e5c62fbb67..59738cbf6e68 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -37,6 +37,10 @@
 #include 
 #include 
 
+#if defined(_FORTIFY_SOURCE) && _FORTIFY_SOURCE > 0
+#include 
+#endif
+
 #ifndef _GID_T_DECLARED
 typedef    __gid_t        gid_t;
 #define    _GID_T_DECLARED
diff --git a/lib/libthr/Makefile b/lib/libthr/Makefile
index a5bf5da44170..85c028f521a1 100644
--- a/lib/libthr/Makefile
+++ b/lib/libthr/Makefile
@@ -11,6 +11,9 @@ LDFLAGS+=    -Wl,--rpath=/usr/lib${COMPAT_libcompat}
 
 .include 
 MK_SSP=    no
+# SSP forced off already implies FORTIFY_SOURCE=0, but we must make sure that
+# one cannot turn it back on.
+FORTIFY_SOURCE=    0
 
 LIB=thr
 SHLIB_MAJOR= 3
diff --git a/libexec/rtld-elf/Makefile b/libexec/rtld-elf/Makefile
index 37c3840538d5..864448ad782a 100644
--- a/libexec/rtld-elf/Makefile
+++ b/libexec/rtld-elf/Makefile
@@ -15,6 +15,10 @@ MK_UBSAN=    no
 
 .include 
 
+# SSP forced off already implies FORTIFY_SOURCE=0, but we must make sure that
+# one cannot turn it back on.
+FORTIFY_SOURCE=    0
+
 .if !defined(NEED_COMPAT)
 CONFS=        libmap.conf
 .endif
diff --git a/share/man/man7/security.7 b/share/man/man7/security.7
index ccbeeb4575ce..2e690e35d534 100644
--- a/share/man/man7/security.7
+++ b/share/man/man7/security.7
@@ -939,6 +939,81 @@ option that SSH allows in its
 .Pa authorized_keys
 file to make the key only usable to entities logging in from specific
 machines.
+.Sh STACK OVERFLOW PROTECTION
+.Fx
+supports stack overflow protection using the Stack Smashing Protector
+.Pq SSP
+compiler feature.
+In userland, SSP adds a per-process randomized canary at the end of every stack
+frame which is checked for corruption upon return from the function.
+In the kernel, a single randomized canary is used globally except on aarch64,
+which has a
+.Dv PERTHREAD_SSP
+.Xr config 8
+option to enable per-thread randomized canaries.
+If 

Re: git: be04fec42638 - main - Import _FORTIFY_SOURCE implementation from NetBSD

2024-05-18 Thread Pedro Giffuni
 Oh no .. please not...
We went into that in a GSoC:
https://wiki.freebsd.org/SummerOfCode2015/FreeBSDLibcSecurityExtensions


Ultimately it proved to be useless since stack-protector-strong.
The NetBSD code was not well adapted to clang either.
Ask me more if you really want to dig into it, but we don't want this.
Pedro.

On Monday, May 13, 2024 at 12:24:13 AM GMT-5, Kyle Evans 
 wrote:  
 
 The branch main has been updated by kevans:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=be04fec42638f30f50b5b55fd8e3634c0fb89928

commit be04fec42638f30f50b5b55fd8e3634c0fb89928
Author:    Kyle Evans 
AuthorDate: 2024-05-13 05:23:49 +
Commit:    Kyle Evans 
CommitDate: 2024-05-13 05:23:49 +

    Import _FORTIFY_SOURCE implementation from NetBSD
    
    This is a mostly-unmodified copy of the various *_chk implementations
    and headers from NetBSD, without yet modifying system headers to start
    actually including them.  A future commit will also apply the needed
    bits to fix ssp/unistd.h.
    
    Reviewed by:    imp, pauamma_gundo.com (both previous versions), kib
    Sponsored by:  Stormshield
    Sponsored by:  Klara, Inc.
    Differential Revision:  https://reviews.freebsd.org/D32306
---
 etc/mtree/BSD.include.dist        |  2 +
 include/Makefile                  |  2 +-
 include/ssp/Makefile              |  6 ++
 include/ssp/ssp.h                  |  91 ++
 include/ssp/stdio.h                |  93 ++
 include/ssp/string.h              | 129 
 include/ssp/strings.h              |  67 +++
 include/ssp/unistd.h              |  54 +++
 lib/libc/secure/Makefile.inc      |  11 
 lib/libc/secure/Symbol.map        |  18 +
 lib/libc/secure/fgets_chk.c        |  54 +++
 lib/libc/secure/gets_chk.c        |  74 +
 lib/libc/secure/memcpy_chk.c      |  53 +++
 lib/libc/secure/memmove_chk.c      |  47 +
 lib/libc/secure/memset_chk.c      |  46 +
 lib/libc/secure/snprintf_chk.c    |  56 
 lib/libc/secure/sprintf_chk.c      |  61 +
 lib/libc/secure/ssp_internal.h    |  37 +++
 lib/libc/secure/stpcpy_chk.c      |  55 
 lib/libc/secure/stpncpy_chk.c      |  53 +++
 lib/libc/secure/strcat_chk.c      |  60 +
 lib/libc/secure/strcpy_chk.c      |  54 +++
 lib/libc/secure/strncat_chk.c      |  70 
 lib/libc/secure/strncpy_chk.c      |  53 +++
 lib/libc/secure/vsnprintf_chk.c    |  49 ++
 lib/libc/secure/vsprintf_chk.c    |  58 
 lib/libssp/Makefile                |  20 +-
 lib/libssp/Symbol.map              |  12 ++--
 lib/libssp/Versions.def            |  5 ++
 lib/libssp/__builtin_object_size.3 | 110 +++
 lib/libssp/fortify_stubs.c        | 131 -
 lib/libssp/ssp.3                  | 130 
 32 files changed, 1621 insertions(+), 140 deletions(-)

diff --git a/etc/mtree/BSD.include.dist b/etc/mtree/BSD.include.dist
index a6bd5880bf61..f8c83d6dde7a 100644
--- a/etc/mtree/BSD.include.dist
+++ b/etc/mtree/BSD.include.dist
@@ -372,6 +372,8 @@
        mac_veriexec
        ..
    ..
+    ssp
+    ..
    sys
        disk
        ..
diff --git a/include/Makefile b/include/Makefile
index 19e6beb95203..32774419f162 100644
--- a/include/Makefile
+++ b/include/Makefile
@@ -4,7 +4,7 @@
 
 PACKAGE=clibs
 CLEANFILES= osreldate.h version
-SUBDIR= arpa protocols rpcsvc rpc xlocale
+SUBDIR= arpa protocols rpcsvc rpc ssp xlocale
 .if ${MACHINE_CPUARCH} == "amd64"
 SUBDIR+=        i386
 INCLUDE_SUBDIRS+=    i386
diff --git a/include/ssp/Makefile b/include/ssp/Makefile
new file mode 100644
index ..dff19f43c920
--- /dev/null
+++ b/include/ssp/Makefile
@@ -0,0 +1,6 @@
+# $FreeBSD$
+
+INCS=    ssp.h stdio.h string.h strings.h unistd.h
+INCSDIR=    ${INCLUDEDIR}/ssp
+
+.include 
diff --git a/include/ssp/ssp.h b/include/ssp/ssp.h
new file mode 100644
index ..35a9aeee02df
--- /dev/null
+++ b/include/ssp/ssp.h
@@ -0,0 +1,91 @@
+/*    $NetBSD: ssp.h,v 1.13 2015/09/03 20:43:47 plunky Exp $    */
+
+/*-
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2006, 2011 The NetBSD Foundation, Inc.
+ * All rights reserved.
+ *
+ * This code is derived from software contributed to The NetBSD Foundation
+ * by Christos Zoulas.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    

Re: git: 3bb5428ad643 - main - rc.conf.5: remove obsolete advice about kld_list

2024-05-18 Thread Warner Losh
On Sat, May 18, 2024, 10:03 AM Colin Percival  wrote:

> On 5/18/24 08:51, Alan Somers wrote:
> > commit 3bb5428ad643e38dd755c8d66c8ac2ae8f7575b5
> >
> >  rc.conf.5: remove obsolete advice about kld_list
> >
> >  Loading modules via kld_list is no longer substantially faster than
> via
> >  loader.conf.
> Where did you benchmark this?  I can believe that performance is similar
> using UEFI, but on systems booting with BIOS doing I/O from the loader is
> very slow because the 20-bit address space forces us to use small I/Os.
>

The performance problem was there was no buffer cache in the boot loader at
all. BIOS is still a little slower, but  ot that much slower on anything
made in the last decade

It used to be like 10-20s faster. Now it's less than a second, even for
BIOS booting. And UEFI is about the same. The block cache is responsible
for most of this. And BIOS doesn't matter so much these days: it is still
in use, but few people use it and the IO speed when they do is DMA speeds
not REPS INB speeds.

Plus it causes some memory fragmentation and there are subtle bugs with it
(like kld_list="uhid ums" working but kld_list="ums uhid" failing).

Warner

-- 
> Colin Percival
> FreeBSD Release Engineering Lead & EC2 platform maintainer
> Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid
>


Re: git: 3bb5428ad643 - main - rc.conf.5: remove obsolete advice about kld_list

2024-05-18 Thread Colin Percival

On 5/18/24 08:51, Alan Somers wrote:

commit 3bb5428ad643e38dd755c8d66c8ac2ae8f7575b5

 rc.conf.5: remove obsolete advice about kld_list
 
 Loading modules via kld_list is no longer substantially faster than via

 loader.conf.

Where did you benchmark this?  I can believe that performance is similar
using UEFI, but on systems booting with BIOS doing I/O from the loader is
very slow because the 20-bit address space forces us to use small I/Os.

--
Colin Percival
FreeBSD Release Engineering Lead & EC2 platform maintainer
Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid



git: 3bb5428ad643 - main - rc.conf.5: remove obsolete advice about kld_list

2024-05-18 Thread Alan Somers
The branch main has been updated by asomers:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=3bb5428ad643e38dd755c8d66c8ac2ae8f7575b5

commit 3bb5428ad643e38dd755c8d66c8ac2ae8f7575b5
Author: Alan Somers 
AuthorDate: 2024-05-18 14:25:10 +
Commit: Alan Somers 
CommitDate: 2024-05-18 15:51:15 +

rc.conf.5: remove obsolete advice about kld_list

Loading modules via kld_list is no longer substantially faster than via
loader.conf.

[skip ci]

MFC after:  2 weeks
Reviewed by:imp
Differential Revision: https://reviews.freebsd.org/D45242
---
 share/man/man5/rc.conf.5 | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/share/man/man5/rc.conf.5 b/share/man/man5/rc.conf.5
index 68d8dade2c6d..51a48341be60 100644
--- a/share/man/man5/rc.conf.5
+++ b/share/man/man5/rc.conf.5
@@ -22,7 +22,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd May 4, 2024
+.Dd May 18, 2024
 .Dt RC.CONF 5
 .Os
 .Sh NAME
@@ -334,10 +334,6 @@ A whitespace-separated list of kernel modules to load 
right after
 the local disks are mounted, without any
 .Pa .ko
 extension or path.
-Loading modules at this point in the boot process is
-much faster than doing it via
-.Pa /boot/loader.conf
-for those modules not necessary for mounting local disks.
 .It Va kldxref_enable
 .Pq Vt bool
 Set to



git: ea916b64125b - main - Remove TCP_SAD optional code now that the sack filter performs this function.

2024-05-18 Thread Randall Stewart
The branch main has been updated by rrs:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=ea916b64125bf43e918db9079532d39f996953ef

commit ea916b64125bf43e918db9079532d39f996953ef
Author: Randall Stewart 
AuthorDate: 2024-05-18 14:57:04 +
Commit: Randall Stewart 
CommitDate: 2024-05-18 14:57:04 +

Remove TCP_SAD optional code now that the sack filter performs this 
function.

With the commit of D44903 we no longer need the SAD option. Instead all 
stacks that
use the sack filter inherit its protection against sack-attack.

Reviewed by: tuexen@
 Differential Revision:https://reviews.freebsd.org/D45216
---
 sys/conf/options  |   1 -
 sys/netinet/tcp_stacks/rack.c | 709 +-
 sys/netinet/tcp_stacks/tcp_rack.h |   9 +-
 sys/netinet/tcp_subr.c|  52 ---
 sys/netinet/tcp_var.h |  13 -
 5 files changed, 16 insertions(+), 768 deletions(-)

diff --git a/sys/conf/options b/sys/conf/options
index 40c24799aa0c..d9bc981232c4 100644
--- a/sys/conf/options
+++ b/sys/conf/options
@@ -242,7 +242,6 @@ TCP_RACKopt_inet.h
 # algorithm that uses "normal" behaviour with SACK's to detect
 # a possible attack. It is strictly experimental at this point.
 #
-TCP_SAD_DETECTION opt_inet.h
 TURNSTILE_PROFILING
 UMTX_PROFILING
 UMTX_CHAINSopt_global.h
diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c
index b0d0f94b8f0e..551ccc1d337a 100644
--- a/sys/netinet/tcp_stacks/rack.c
+++ b/sys/netinet/tcp_stacks/rack.c
@@ -536,7 +536,7 @@ static int32_t rack_output(struct tcpcb *tp);
 static uint32_t
 rack_proc_sack_blk(struct tcpcb *tp, struct tcp_rack *rack,
 struct sackblk *sack, struct tcpopt *to, struct rack_sendmap **prsm,
-uint32_t cts, int *no_extra, int *moved_two, uint32_t segsiz);
+uint32_t cts, uint32_t segsiz);
 static void rack_post_recovery(struct tcpcb *tp, uint32_t th_seq);
 static void rack_remxt_tmr(struct tcpcb *tp);
 static int rack_set_sockopt(struct tcpcb *tp, struct sockopt *sopt);
@@ -2752,8 +2752,6 @@ rack_log_retran_reason(struct tcp_rack *rack, struct 
rack_sendmap *rsm, uint32_t
union tcp_log_stackspecific log;
struct timeval tv;
 
-   if (rack->sack_attack_disable > 0)
-   goto log_anyway;
if ((mod != 1) && (rack_verbose_logging == 0))  {
/*
 * We get 3 values currently for mod
@@ -2766,7 +2764,6 @@ rack_log_retran_reason(struct tcp_rack *rack, struct 
rack_sendmap *rsm, uint32_t
 */
return;
}
-log_anyway:
memset(_bbr, 0, sizeof(log.u_bbr));
log.u_bbr.flex1 = tsused;
log.u_bbr.flex2 = thresh;
@@ -3007,13 +3004,8 @@ rack_log_rtt_sample(struct tcp_rack *rack, uint32_t rtt)
/* Convert our ms to a microsecond */
memset(, 0, sizeof(log));
log.u_bbr.flex1 = rtt;
-   log.u_bbr.flex2 = rack->r_ctl.ack_count;
-   log.u_bbr.flex3 = rack->r_ctl.sack_count;
-   log.u_bbr.flex4 = rack->r_ctl.sack_noextra_move;
-   log.u_bbr.flex5 = rack->r_ctl.sack_moved_extra;
log.u_bbr.flex6 = rack->rc_tp->t_rxtcur;
log.u_bbr.flex7 = 1;
-   log.u_bbr.flex8 = rack->sack_attack_disable;
log.u_bbr.timeStamp = tcp_get_usecs();
log.u_bbr.inflight = ctf_flight_size(rack->rc_tp, 
rack->r_ctl.rc_sacked);
log.u_bbr.pkts_out = rack->r_ctl.rc_out_at_rto;
@@ -3144,7 +3136,6 @@ rack_log_type_bbrsnd(struct tcp_rack *rack, uint32_t len, 
uint32_t slot, uint32_
else
log.u_bbr.flex2 = rack->r_ctl.rc_prr_sndcnt;
log.u_bbr.flex4 = rack->r_ctl.rc_hpts_flags;
-   log.u_bbr.flex5 = rack->r_ctl.ack_during_sd;
log.u_bbr.flex6 = line;
log.u_bbr.flex7 = (0x & rack->r_ctl.rc_hpts_flags);
log.u_bbr.flex8 = rack->rc_in_persist;
@@ -3406,40 +3397,6 @@ rack_log_to_prr(struct tcp_rack *rack, int frm, int 
orig_cwnd, int line)
}
 }
 
-#ifdef TCP_SAD_DETECTION
-static void
-rack_log_sad(struct tcp_rack *rack, int event)
-{
-   if (tcp_bblogging_on(rack->rc_tp)) {
-   union tcp_log_stackspecific log;
-   struct timeval tv;
-
-   memset(_bbr, 0, sizeof(log.u_bbr));
-   log.u_bbr.flex1 = rack->r_ctl.sack_count;
-   log.u_bbr.flex2 = rack->r_ctl.ack_count;
-   log.u_bbr.flex3 = rack->r_ctl.sack_moved_extra;
-   log.u_bbr.flex4 = rack->r_ctl.sack_noextra_move;
-   log.u_bbr.flex5 = rack->r_ctl.rc_num_maps_alloced;
-   log.u_bbr.flex6 = tcp_sack_to_ack_thresh;
-   log.u_bbr.pkts_out = tcp_sack_to_move_thresh;
-   log.u_bbr.lt_epoch = 

git: 97aedd3395b4 - main - devctl: Disable the boottime optimization of suppressing NOMATCH

2024-05-18 Thread Warner Losh
The branch main has been updated by imp:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=97aedd3395b4e9b017e29823096771aff0835ff0

commit 97aedd3395b4e9b017e29823096771aff0835ff0
Author: Warner Losh 
AuthorDate: 2024-05-18 13:07:16 +
Commit: Warner Losh 
CommitDate: 2024-05-18 13:07:16 +

devctl: Disable the boottime optimization of suppressing NOMATCH

The usb bus code (uhub) doens't present the same information to devctl
as it does to the NOMATCH events it generats. As such, devmatch fails to
find USB devices on boot when NOMATCH events are optimized out. Since
the savings of boot time is relatively trivial for all but the most
demanding boot environments, disable it by default until this issue is
fixed.

Fixes: 6437872c1d66
MFC After: 1 minute
Sponsored by:   Netflix
---
 sys/kern/kern_devctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/kern/kern_devctl.c b/sys/kern/kern_devctl.c
index 0dd05a49c9ad..602b82105525 100644
--- a/sys/kern/kern_devctl.c
+++ b/sys/kern/kern_devctl.c
@@ -89,7 +89,7 @@ static int sysctl_devctl_queue(SYSCTL_HANDLER_ARGS);
 static int devctl_queue_length = DEVCTL_DEFAULT_QUEUE_LEN;
 SYSCTL_PROC(_hw_bus, OID_AUTO, devctl_queue, CTLTYPE_INT | CTLFLAG_RWTUN |
 CTLFLAG_MPSAFE, NULL, 0, sysctl_devctl_queue, "I", "devctl queue length");
-static bool nomatch_enabled = false;
+static bool nomatch_enabled = true;
 SYSCTL_BOOL(_hw_bus, OID_AUTO, devctl_nomatch_enabled, CTLFLAG_RWTUN,
 _enabled, 0, "enable nomatch events");