Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Bruce Evans]

On Tue, 23 Jan 2018, Pedro Giffuni wrote:


On 23/01/2018 14:08, Conrad Meyer wrote:

Hi Pedro,

On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni  wrote:

Author: pfg
Date: Sun Jan 21 15:42:36 2018
New Revision: 328218
URL: https://svnweb.freebsd.org/changeset/base/328218

Log:
   Revert r327828, r327949, r327953, r328016-r328026, r328041:
   Uses of mallocarray(9).

   The use of mallocarray(9) has rocketed the required swap to build 
FreeBSD.
   This is likely caused by the allocation size attributes which put extra 
pressure

   on the compiler.

I'm confused about this change.  Wouldn't it be better to remove the
annotation/attributes from mallocarray() than to remove the protection
against overflow?


It would be better to remove mallocarray().

Not in my opinion: it would be better to detect such overflows at compile 
time (or through a static analyzer) than to have late notification though 
panics. The blind use of mallocarray(9) is probably a mistake also: we 
shouldn't use it unless there is some real risk of overflow.


Excellent!  There is no real risk of overflow except in cases that are
broken anyway.  The overflow checks in mallocarray() are insufficient even
for detecting overflow and don't detect most broken cases when they are
sufficient.  This leaves no cases where even non-blind use of it does any
good.


   (If the compiler is fixed in the future to not use
excessive memory with these attributes, they can be conditionalized on
compiler version, of course.)


All in all, the compiler is not provably wrong: it's just using more swap 
space, which is rather inconvenient for small platforms but not necessarily 
wrong.


I don't know why the compiler uses more swap space, but the general brokenness
of mallocarray() is obvious.

Callers must somehow ensure that the allocation size is not too large.
Too large is closer to 64K than SIZE_MAX for most allocations.  Some
bloatware allocates a few MB, but malloc() is rarely used for that.
vmstat -m has been broken to not show a summary of sizes at the end,
but on freefall now it seems to shows a maximum malloc() size of 64K,
and vmstat -z confirms this by not showing any malloc() bucket sizes
larger than 64K, and in fact kern_malloc.c only has statically allocatd
bucket sizes up to 64K.  (vmstat -z never had a summary at the end to
break).  Much larger allocations are mostly done using k*alloc(), and
slightly larger allocations are usually done using UMA.  zfs does
zillions of allocations using UMA, and the largest one seems to be 16M.

If the caller doesn't check, this gives a Denial of Service security hole
if the size and count are controlled by userland.  If the size and count
are controlled by the kernel, then the overflow check is not very useful.
It is less useful than most KASSERT()s which are left out of production
kernels.  The caller must keep its allocation sizes not much more than
64K, and once it does that it is unlikely to make them larger than SIZE_MAX
sometimes.

The overflow checks in mallocarray have many type errors so they don't
always work:

X Modified: head/share/man/man9/malloc.9
X ==
X --- head/share/man/man9/malloc.9  Sun Jan  7 10:29:15 2018
(r327673)
X +++ head/share/man/man9/malloc.9  Sun Jan  7 13:21:01 2018
(r327674)
X @@ -45,6 +45,8 @@
X  .In sys/malloc.h
X  .Ft void *
X  .Fn malloc "unsigned long size" "struct malloc_type *type" "int flags"
X +.Ft void *
X +.Fn mallocarray "size_t nmemb" "size_t size" "struct malloc_type *type" "int 
flags"

One type error is already obvious.  malloc(9)'s arg doesn't have type size_t
like malloc(3)'s arg.  The arg type is u_long in the kernel.  mallocarray()'s
types are inconsistent with this.

size_t happens to have the same representation as u_long on all supported
arches, so this doesn't cause many problems now.  On 32-bit arches, size_t
hs type u_int.  u_int has smaller rank than u_long, so compilers could
reasonably complain that converting a u_long to a size_t is a downcast.
They shouldn't complain that converting a size_t to a u_long to pass it
to malloc() is an upcast, so there is no problem in typical sloppy code
that uses size_t for sizes and passes these to malloc().  More careful
ciode would use u_long for size to pass to malloc() and compiler's might
complain about downcasting to pass to mallocarray() instead.

Otherwise (on exotic machines with size_t larger than u_long), passing
size_t's to malloc() is a bug unless they values have been checked to
be <= ULONG_MAX, and mallocarrary()'s inconsistent API expands this bug.

X Modified: head/sys/kern/kern_malloc.c
X ==
X --- head/sys/kern/kern_malloc.c   Sun Jan  7 10:29:15 2018
(r327673)
X +++ head/sys/kern/kern_malloc.c   Sun Jan  7 13:21:01 2018
(r327674)
X @@ -532,6 +533,22 @@ malloc(unsigned long size, struct malloc_typ

svn commit: r328321 - head/sys/net

[Steven Hartland]

Author: smh
Date: Wed Jan 24 10:13:14 2018
New Revision: 328321
URL: https://svnweb.freebsd.org/changeset/base/328321

Log:
  Added missing CTLFLAG_VNET to lacp default_strict_mode
  
  Added CTLFLAG_VNET to net.link.lagg.lacp.default_strict_mode which was missed
  in r290450.
  
  Reported by:  julian@
  MFC after:1 week
  Sponsored by: Multiplay

Modified:
  head/sys/net/ieee8023ad_lacp.c

Modified: head/sys/net/ieee8023ad_lacp.c
==
--- head/sys/net/ieee8023ad_lacp.c  Wed Jan 24 07:54:05 2018
(r328320)
+++ head/sys/net/ieee8023ad_lacp.c  Wed Jan 24 10:13:14 2018
(r328321)
@@ -201,8 +201,8 @@ SYSCTL_INT(_net_link_lagg_lacp, OID_AUTO, debug, CTLFL
 &VNET_NAME(lacp_debug), 0, "Enable LACP debug logging (1=debug, 2=trace)");
 
 static VNET_DEFINE(int, lacp_default_strict_mode) = 1;
-SYSCTL_INT(_net_link_lagg_lacp, OID_AUTO, default_strict_mode, CTLFLAG_RWTUN,
-&VNET_NAME(lacp_default_strict_mode), 0,
+SYSCTL_INT(_net_link_lagg_lacp, OID_AUTO, default_strict_mode,
+CTLFLAG_RWTUN | CTLFLAG_VNET, &VNET_NAME(lacp_default_strict_mode), 0,
 "LACP strict protocol compliance default");
 
 #define LACP_DPRINTF(a) if (V_lacp_debug & 0x01) { lacp_dprintf a ; }
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328305 - head/lib/libcxxrt

[Dimitry Andric]

On 23 Jan 2018, at 23:41, Ed Maste  wrote:
> 
> Author: emaste
> Date: Tue Jan 23 22:41:13 2018
> New Revision: 328305
> URL: https://svnweb.freebsd.org/changeset/base/328305
> 
> Log:
>  libcxxrt: Move mangled symbols out of extern "C++" in Version.map
> 
>  r260553 added a number of mangled C++ symbols to Version.map inside of
>  an existing `extern "C++"` block.
> 
>  ld.bfd 2.17.50 treats `extern "C++"` permissively and will match both
>  mangled and demangled symbols against the strings in the version map
>  block.  ld.lld interprets `extern "C++"` strictly, and matches only
>  demangled symbols.
> 
>  I believe lld's behaviour is correct.  Contemporary versions of ld.bfd
>  also behave as lld does, so move the mangled symbols out of the
>  `extern "C++"` block.
> 
>  PR:  225128, 185663
>  MFC after:   1 week
>  Sponsored by:The FreeBSD Foundation
> 
> Modified:
>  head/lib/libcxxrt/Version.map
> 
> Modified: head/lib/libcxxrt/Version.map
> ==
> --- head/lib/libcxxrt/Version.map Tue Jan 23 22:18:45 2018
> (r328304)
> +++ head/lib/libcxxrt/Version.map Tue Jan 23 22:41:13 2018
> (r328305)
> @@ -112,19 +112,6 @@ CXXABI_1.3 {
> "typeinfo for void";
> "typeinfo for wchar_t const*";
> "typeinfo for wchar_t";
> -# C++11 typeinfo not understood by our linker
> -# std::nullptr_t
> -_ZTIDn;_ZTIPDn;_ZTIPKDn;

Maybe, for readability, we should change these to unmangled form instead?

I didn't really understand why the file had mixed mangled and unmangled forms.

-Dimitry



signature.asc
Description: Message signed with OpenPGP

svn commit: r328325 - head/sys/powerpc/powerpc

[Wojciech Macek]

Author: wma
Date: Wed Jan 24 12:01:32 2018
New Revision: 328325
URL: https://svnweb.freebsd.org/changeset/base/328325

Log:
  PPC: Add KASSERT in intrcnt_add which checks for buffer overflow
  
  Authored by:   Patryk Duda 
  Submitted by:  Wojciech Macek 
  Obtained from: Semihalf
  Sponsored by:  IBM, QCM Technologies

Modified:
  head/sys/powerpc/powerpc/intr_machdep.c

Modified: head/sys/powerpc/powerpc/intr_machdep.c
==
--- head/sys/powerpc/powerpc/intr_machdep.c Wed Jan 24 11:03:18 2018
(r328324)
+++ head/sys/powerpc/powerpc/intr_machdep.c Wed Jan 24 12:01:32 2018
(r328325)
@@ -178,6 +178,8 @@ intrcnt_add(const char *name, u_long **countp)
int idx;
 
idx = atomic_fetchadd_int(&intrcnt_index, 1);
+   KASSERT(idx < INTR_VECTORS, ("intrcnt_add: Interrupt counter index "
+   "reached INTR_VECTORS"));
*countp = &intrcnt[idx];
intrcnt_setname(name, idx);
 }
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328326 - head/sys/netpfil/ipfw

[Andrey V. Elsukov]

Author: ae
Date: Wed Jan 24 12:40:28 2018
New Revision: 328326
URL: https://svnweb.freebsd.org/changeset/base/328326

Log:
  When IPv6 packet is handled by O_REJECT opcode, convert ICMP code
  specified in the arg1 into ICMPv6 destination unreachable code according
  to RFC7915.
  
  Obtained from:Yandex LLC
  MFC after:2 weeks
  Sponsored by: Yandex LLC

Modified:
  head/sys/netpfil/ipfw/ip_fw2.c

Modified: head/sys/netpfil/ipfw/ip_fw2.c
==
--- head/sys/netpfil/ipfw/ip_fw2.c  Wed Jan 24 12:01:32 2018
(r328325)
+++ head/sys/netpfil/ipfw/ip_fw2.c  Wed Jan 24 12:40:28 2018
(r328326)
@@ -821,6 +821,32 @@ is_icmp6_query(int icmp6_type)
return (0);
 }
 
+static int
+map_icmp_unreach(int code)
+{
+
+   /* RFC 7915 p4.2 */
+   switch (code) {
+   case ICMP_UNREACH_NET:
+   case ICMP_UNREACH_HOST:
+   case ICMP_UNREACH_SRCFAIL:
+   case ICMP_UNREACH_NET_UNKNOWN:
+   case ICMP_UNREACH_HOST_UNKNOWN:
+   case ICMP_UNREACH_TOSNET:
+   case ICMP_UNREACH_TOSHOST:
+   return (ICMP6_DST_UNREACH_NOROUTE);
+   case ICMP_UNREACH_PORT:
+   return (ICMP6_DST_UNREACH_NOPORT);
+   default:
+   /*
+* Map the rest of codes into admit prohibited.
+* XXX: unreach proto should be mapped into ICMPv6
+* parameter problem, but we use only unreach type.
+*/
+   return (ICMP6_DST_UNREACH_ADMIN);
+   }
+}
+
 static void
 send_reject6(struct ip_fw_args *args, int code, u_int hlen, struct ip6_hdr 
*ip6)
 {
@@ -2831,9 +2857,12 @@ do { 
\
(proto != IPPROTO_ICMPV6 ||
 (is_icmp6_query(icmp6_type) == 1)) &&
!(m->m_flags & (M_BCAST|M_MCAST)) &&
-   
!IN6_IS_ADDR_MULTICAST(&args->f_id.dst_ip6)) {
-   send_reject6(
-   args, cmd->arg1, hlen,
+   !IN6_IS_ADDR_MULTICAST(
+   &args->f_id.dst_ip6)) {
+   send_reject6(args,
+   cmd->opcode == O_REJECT ?
+   map_icmp_unreach(cmd->arg1):
+   cmd->arg1, hlen,
(struct ip6_hdr *)ip);
m = args->m;
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328305 - head/lib/libcxxrt

[Ed Maste]

On 24 January 2018 at 06:51, Dimitry Andric  wrote:
>> -# C++11 typeinfo not understood by our linker
>> -# std::nullptr_t
>> -_ZTIDn;_ZTIPDn;_ZTIPKDn;
>
> Maybe, for readability, we should change these to unmangled form instead?

I believe they're mangled because ld.bfd 2.17.50 cannot demangle them.
I think we have to keep them like this until we're fully committed to
using only in-tree lld or external toolchain linkers across platforms.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328329 - head/sys/compat/linuxkpi/common/src

[Hans Petter Selasky]

Author: hselasky
Date: Wed Jan 24 13:37:07 2018
New Revision: 328329
URL: https://svnweb.freebsd.org/changeset/base/328329

Log:
  Properly implement the "id" callback argument in the "idr_for_each" function
  in the LinuxKPI. The old implementation assumed only one IDR layer was 
present.
  Take additional IDR layers into account when computing the "id" value.
  
  MFC after:1 week
  Found by: Karthik Palanichamy 
  Tested by:Karthik Palanichamy 
  Sponsored by: Mellanox Technologies

Modified:
  head/sys/compat/linuxkpi/common/src/linux_idr.c

Modified: head/sys/compat/linuxkpi/common/src/linux_idr.c
==
--- head/sys/compat/linuxkpi/common/src/linux_idr.c Wed Jan 24 13:12:21 
2018(r328328)
+++ head/sys/compat/linuxkpi/common/src/linux_idr.c Wed Jan 24 13:37:07 
2018(r328329)
@@ -680,7 +680,7 @@ idr_alloc_cyclic(struct idr *idr, void *ptr, int start
 }
 
 static int
-idr_for_each_layer(struct idr_layer *il, int layer,
+idr_for_each_layer(struct idr_layer *il, int offset, int layer,
 int (*f)(int id, void *p, void *data), void *data)
 {
int i, err;
@@ -691,7 +691,7 @@ idr_for_each_layer(struct idr_layer *il, int layer,
for (i = 0; i < IDR_SIZE; i++) {
if (il->ary[i] == NULL)
continue;
-   err = f(i, il->ary[i],  data);
+   err = f(i + offset, il->ary[i],  data);
if (err)
return (err);
}
@@ -700,7 +700,8 @@ idr_for_each_layer(struct idr_layer *il, int layer,
for (i = 0; i < IDR_SIZE; i++) {
if (il->ary[i] == NULL)
continue;
-   err = idr_for_each_layer(il->ary[i], layer - 1, f, data);
+   err = idr_for_each_layer(il->ary[i],
+   (i + offset) * IDR_SIZE, layer - 1, f, data);
if (err)
return (err);
}
@@ -711,7 +712,7 @@ idr_for_each_layer(struct idr_layer *il, int layer,
 int
 idr_for_each(struct idr *idp, int (*f)(int id, void *p, void *data), void 
*data)
 {
-   return (idr_for_each_layer(idp->top, idp->layers - 1, f, data));
+   return (idr_for_each_layer(idp->top, 0, idp->layers - 1, f, data));
 }
 
 int
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328320 - head/sys/kern

[Hans Petter Selasky]

On 01/24/18 08:54, Wojciech Macek wrote:

Author: wma
Date: Wed Jan 24 07:54:05 2018
New Revision: 328320
URL: https://svnweb.freebsd.org/changeset/base/328320

Log:
   ULE: provide defaults to ts_cpu
   
   Fix a bug when the system has no CPU 0. When created, threads were implicitly assigned to CPU 0.

   This had no practical effect since a real CPU was chosen immediately by the 
scheduler. However,
   on systems without a CPU 0, sched_ule attempted to access the scheduler queue of the 
"old" CPU
   when assigned the initial choice of the old one. This caused an attempt to 
use illegal memory
   and a crash (or, more usually, a deadlock). Fix this by assigned new threads 
to the BSP
   explicitly and add some asserts to see that this problem does not recur.
   
   Authored by:   Nathan Whitehorn 

   Submitted by:  Wojciech Macek 
   Obtained from: Semihalf
   Differential revision: https://reviews.freebsd.org/D13932

Modified:
   head/sys/kern/sched_ule.c

Modified: head/sys/kern/sched_ule.c
==
--- head/sys/kern/sched_ule.c   Wed Jan 24 07:01:44 2018(r328319)
+++ head/sys/kern/sched_ule.c   Wed Jan 24 07:54:05 2018(r328320)
@@ -1405,6 +1405,7 @@ sched_setup(void *dummy)
  
  	/* Add thread0's load since it's running. */

TDQ_LOCK(tdq);
+   td_get_sched(&thread0)->ts_cpu = curcpu; /* Something valid to start */
thread0.td_lock = TDQ_LOCKPTR(TDQ_SELF());
tdq_load_add(tdq, &thread0);
tdq->tdq_lowpri = thread0.td_priority;
@@ -2454,6 +2455,7 @@ sched_add(struct thread *td, int flags)
 * Pick the destination cpu and if it isn't ours transfer to the
 * target cpu.
 */
+   td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to start */
cpu = sched_pickcpu(td, flags);
tdq = sched_setcpu(td, cpu, flags);
tdq_add(tdq, td, flags);




Hi Wojciech,

I believe this chunk is wrong and the same like in r326218. See the 
explanation for r326376 as pasted below. Can you please revert?


Further refer to existing discussions about "r326218" on the SVN commit 
list.


Maybe you missed the point of r326376 when integrating?

--HPS


Revision 326376 - (view) (download) (annotate) - [select for diffs]
Modified Wed Nov 29 23:28:40 2017 UTC (7 weeks, 6 days ago) by hselasky
File length: 80183 byte(s)
Diff to previous 326271

The sched_add() function is not only used when the thread is initially
started, but also by the turnstiles to mark a thread as runnable for
all locks, for instance sleepqueues do:
setrunnable()->sched_wakeup()->sched_add()

In r326218 code was added to allow booting from non-zero CPU numbers
by setting the ts_cpu field inside the ULE scheduler's sched_add()
function. This had an undesired side-effect that prior sched_pin() and
sched_bind() calls got disregarded. This patch fixes the
initialization of the ts_cpu field for the ULE scheduler to only
happen once when the initial thread is constructed during system
init. Forking will then later on ensure that a valid ts_cpu value gets
copied to all children.

Reviewed by:jhb, kib
Discussed with: nwhitehorn
MFC after:  1 month
Differential revision:  https://reviews.freebsd.org/D13298
Sponsored by:   Mellanox Technologies




___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328330 - head/sys/kern

[Wojciech Macek]

Author: wma
Date: Wed Jan 24 13:57:01 2018
New Revision: 328330
URL: https://svnweb.freebsd.org/changeset/base/328330

Log:
  Reverting r328320

Modified:
  head/sys/kern/sched_ule.c

Modified: head/sys/kern/sched_ule.c
==
--- head/sys/kern/sched_ule.c   Wed Jan 24 13:37:07 2018(r328329)
+++ head/sys/kern/sched_ule.c   Wed Jan 24 13:57:01 2018(r328330)
@@ -1405,7 +1405,6 @@ sched_setup(void *dummy)
 
/* Add thread0's load since it's running. */
TDQ_LOCK(tdq);
-   td_get_sched(&thread0)->ts_cpu = curcpu; /* Something valid to start */
thread0.td_lock = TDQ_LOCKPTR(TDQ_SELF());
tdq_load_add(tdq, &thread0);
tdq->tdq_lowpri = thread0.td_priority;
@@ -2455,7 +2454,6 @@ sched_add(struct thread *td, int flags)
 * Pick the destination cpu and if it isn't ours transfer to the
 * target cpu.
 */
-   td_get_sched(td)->ts_cpu = curcpu; /* Pick something valid to start */
cpu = sched_pickcpu(td, flags);
tdq = sched_setcpu(td, cpu, flags);
tdq_add(tdq, td, flags);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328330 - head/sys/kern

[Cy Schubert]

In message <201801241357.w0odv1tm043...@repo.freebsd.org>, Wojciech Macek 
write
s:
> Author: wma
> Date: Wed Jan 24 13:57:01 2018
> New Revision: 328330
> URL: https://svnweb.freebsd.org/changeset/base/328330
>
> Log:
>   Reverting r328320
>
> Modified:
>   head/sys/kern/sched_ule.c
>
> Modified: head/sys/kern/sched_ule.c

Why? What happened?


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328330 - head/sys/kern

[Hans Petter Selasky]

On 01/24/18 15:00, Cy Schubert wrote:

Why? What happened?



Hi,

See r326376 . This patch is out of date :-)

--HPS

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328331 - head/etc

[Dmitry Marakasov]

Author: amdmi3 (ports committer)
Date: Wed Jan 24 14:15:06 2018
New Revision: 328331
URL: https://svnweb.freebsd.org/changeset/base/328331

Log:
  Support configuring arbitrary limits(1) for any daemon in rc.conf
  
  Usage is ${name}_limits, and the argument is any flags accepted by
  limits(1), such as `-n 100' (e.g. only allow 100 open files).
  
  Approved by:  cy
  Differential Revision:https://reviews.freebsd.org/D14015

Modified:
  head/etc/rc.subr

Modified: head/etc/rc.subr
==
--- head/etc/rc.subrWed Jan 24 13:57:01 2018(r328330)
+++ head/etc/rc.subrWed Jan 24 14:15:06 2018(r328331)
@@ -775,6 +775,8 @@ check_startmsgs()
 #
 #  ${name}_login_class n   Login class to use, else "daemon".
 #
+#  ${name}_limits  n   limits(1) to apply to ${command}.
+#
 #  ${rc_arg}_cmd   n   If set, use this as the method when invoked;
 #  Otherwise, use default command (see below)
 #
@@ -952,7 +954,7 @@ run_rc_command()
_group=\$${name}_group  _groups=\$${name}_groups \
_fib=\$${name}_fib  _env=\$${name}_env \
_prepend=\$${name}_prepend  
_login_class=\${${name}_login_class:-daemon} \
-   _oomprotect=\$${name}_oomprotect
+   _limits=\$${name}_limits_oomprotect=\$${name}_oomprotect
 
if [ -n "$_user" ]; then# unset $_user if running as that user
if [ "$_user" = "$(eval $IDCMD)" ]; then
@@ -1073,7 +1075,7 @@ $command $rc_flags $command_args"
fi
 
# Prepend default limits
-   _doit="$_cd limits -C $_login_class $_doit"
+   _doit="$_cd limits -C $_login_class $_limits $_doit"
 
# run the full command
#
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328332 - in head: contrib/libarchive/cat contrib/libarchive/cat/test contrib/libarchive/libarchive contrib/libarchive/libarchive/test contrib/libarchive/tar/test lib/libarchive/tests u...

[Martin Matuska]

Author: mm
Date: Wed Jan 24 14:24:17 2018
New Revision: 328332
URL: https://svnweb.freebsd.org/changeset/base/328332

Log:
  MFV r328323,328324:
  Sync libarchive with vendor.
  
  Relevant vendor changes:
PR #893: delete dead ppmd7 alloc callbacks
PR #904: Fix archive freeing bug in bsdcat
PR #961: Fix ZIP format names
PR #962: Don't modify attributes for existing directories
 when ARCHIVE_EXTRACT_NO_OVERWRITE is set
PR #964: Fix -Werror=implicit-fallthrough= for GCC 7
PR #970: zip: Allow backslash as path separator
  
  MFC after:1 week

Added:
  head/contrib/libarchive/cat/test/test_stdin.c
 - copied unchanged from r328324, 
vendor/libarchive/dist/cat/test/test_stdin.c
  head/contrib/libarchive/libarchive/test/test_compat_zip_8.zip.uu
 - copied unchanged from r328324, 
vendor/libarchive/dist/libarchive/test/test_compat_zip_8.zip.uu
Modified:
  head/contrib/libarchive/cat/bsdcat.c
  head/contrib/libarchive/libarchive/archive_acl.c
  head/contrib/libarchive/libarchive/archive_disk_acl_freebsd.c
  head/contrib/libarchive/libarchive/archive_match.c
  head/contrib/libarchive/libarchive/archive_platform.h
  head/contrib/libarchive/libarchive/archive_ppmd7.c
  head/contrib/libarchive/libarchive/archive_ppmd7_private.h
  head/contrib/libarchive/libarchive/archive_ppmd_private.h
  head/contrib/libarchive/libarchive/archive_read.c
  head/contrib/libarchive/libarchive/archive_read_disk_posix.c
  head/contrib/libarchive/libarchive/archive_read_support_format_7zip.c
  head/contrib/libarchive/libarchive/archive_read_support_format_mtree.c
  head/contrib/libarchive/libarchive/archive_read_support_format_rar.c
  head/contrib/libarchive/libarchive/archive_read_support_format_tar.c
  head/contrib/libarchive/libarchive/archive_read_support_format_zip.c
  head/contrib/libarchive/libarchive/archive_util.c
  head/contrib/libarchive/libarchive/archive_virtual.c
  head/contrib/libarchive/libarchive/archive_write.c
  head/contrib/libarchive/libarchive/archive_write_disk_posix.c
  head/contrib/libarchive/libarchive/archive_write_set_format_7zip.c
  head/contrib/libarchive/libarchive/test/read_open_memory.c
  head/contrib/libarchive/libarchive/test/test.h
  head/contrib/libarchive/libarchive/test/test_acl_platform_nfs4.c
  head/contrib/libarchive/libarchive/test/test_compat_zip.c
  head/contrib/libarchive/libarchive/test/test_read_format_zip.c
  head/contrib/libarchive/libarchive/test/test_write_disk_perms.c
  head/contrib/libarchive/tar/test/test_option_acls.c
  head/lib/libarchive/tests/Makefile
  head/usr.bin/bsdcat/tests/Makefile
Directory Properties:
  head/contrib/libarchive/   (props changed)

Modified: head/contrib/libarchive/cat/bsdcat.c
==
--- head/contrib/libarchive/cat/bsdcat.cWed Jan 24 14:15:06 2018
(r328331)
+++ head/contrib/libarchive/cat/bsdcat.cWed Jan 24 14:24:17 2018
(r328332)
@@ -70,6 +70,12 @@ version(void)
 void
 bsdcat_next(void)
 {
+   if (a != NULL) {
+   if (archive_read_close(a) != ARCHIVE_OK)
+   bsdcat_print_error();
+   archive_read_free(a);
+   }
+
a = archive_read_new();
archive_read_support_filter_all(a);
archive_read_support_format_empty(a);
@@ -100,8 +106,10 @@ bsdcat_read_to_stdout(const char* filename)
;
else if (archive_read_data_into_fd(a, 1) != ARCHIVE_OK)
bsdcat_print_error();
-   if (archive_read_free(a) != ARCHIVE_OK)
+   if (archive_read_close(a) != ARCHIVE_OK)
bsdcat_print_error();
+   archive_read_free(a);
+   a = NULL;
 }
 
 int
@@ -135,15 +143,14 @@ main(int argc, char **argv)
if (*bsdcat->argv == NULL) {
bsdcat_current_path = "";
bsdcat_read_to_stdout(NULL);
-   } else
+   } else {
while (*bsdcat->argv) {
bsdcat_current_path = *bsdcat->argv++;
bsdcat_read_to_stdout(bsdcat_current_path);
bsdcat_next();
}
-
-   if (a != NULL)
-   archive_read_free(a);
+   archive_read_free(a); /* Help valgrind & friends */
+   }
 
exit(exit_status);
 }

Copied: head/contrib/libarchive/cat/test/test_stdin.c (from r328324, 
vendor/libarchive/dist/cat/test/test_stdin.c)
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/contrib/libarchive/cat/test/test_stdin.c   Wed Jan 24 14:24:17 
2018(r328332, copy of r328324, 
vendor/libarchive/dist/cat/test/test_stdin.c)
@@ -0,0 +1,42 @@
+/*-
+ * Copyright (c) 2017 Sean Purcell
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *

Re: svn commit: r328330 - head/sys/kern

[Cy Schubert]

In message <82d52ee3-1c2a-2399-0142-0027b4370...@selasky.org>, Hans Petter 
Sela
sky writes:
> On 01/24/18 15:00, Cy Schubert wrote:
> > Why? What happened?
> > 
>
> Hi,
>
> See r326376 . This patch is out of date :-)

This makes my point. Someone browsing through the log will not be able to 
determine without dissecting the code why.


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Wed, Jan 24, 2018 at 1:50 AM, Bruce Evans  wrote:

> On Tue, 23 Jan 2018, Pedro Giffuni wrote:
>
> On 23/01/2018 14:08, Conrad Meyer wrote:
>>
>>> Hi Pedro,
>>>
>>> On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni 
>>> wrote:
>>>
 Author: pfg
 Date: Sun Jan 21 15:42:36 2018
 New Revision: 328218
 URL: https://svnweb.freebsd.org/changeset/base/328218

 Log:
Revert r327828, r327949, r327953, r328016-r328026, r328041:
Uses of mallocarray(9).

The use of mallocarray(9) has rocketed the required swap to build
 FreeBSD.
This is likely caused by the allocation size attributes which put
 extra pressure
on the compiler.

>>> I'm confused about this change.  Wouldn't it be better to remove the
>>> annotation/attributes from mallocarray() than to remove the protection
>>> against overflow?
>>>
>>
> It would be better to remove mallocarray().


I agree completely. It doesn't do what you think it is doing, for all the
reasons that Bruce outlines. We thought it was a bad idea when it came up 2
years ago and nothing has really changed.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328336 - head/usr.sbin/iscsid

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:34:37 2018
New Revision: 328336
URL: https://svnweb.freebsd.org/changeset/base/328336

Log:
  Add missing SPDX identifier in iscsid(8).
  
  MFC after:2 weeks

Modified:
  head/usr.sbin/iscsid/chap.c

Modified: head/usr.sbin/iscsid/chap.c
==
--- head/usr.sbin/iscsid/chap.c Wed Jan 24 16:33:33 2018(r328335)
+++ head/usr.sbin/iscsid/chap.c Wed Jan 24 16:34:37 2018(r328336)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328335 - head/usr.sbin/uefisign

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:33:33 2018
New Revision: 328335
URL: https://svnweb.freebsd.org/changeset/base/328335

Log:
  Add SPDX identifiers for uefisign(8) sources.
  
  MFC after:2 weeks

Modified:
  head/usr.sbin/uefisign/child.c
  head/usr.sbin/uefisign/magic.h
  head/usr.sbin/uefisign/pe.c
  head/usr.sbin/uefisign/uefisign.c
  head/usr.sbin/uefisign/uefisign.h

Modified: head/usr.sbin/uefisign/child.c
==
--- head/usr.sbin/uefisign/child.c  Wed Jan 24 15:16:17 2018
(r328334)
+++ head/usr.sbin/uefisign/child.c  Wed Jan 24 16:33:33 2018
(r328335)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/uefisign/magic.h
==
--- head/usr.sbin/uefisign/magic.h  Wed Jan 24 15:16:17 2018
(r328334)
+++ head/usr.sbin/uefisign/magic.h  Wed Jan 24 16:33:33 2018
(r328335)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/uefisign/pe.c
==
--- head/usr.sbin/uefisign/pe.c Wed Jan 24 15:16:17 2018(r328334)
+++ head/usr.sbin/uefisign/pe.c Wed Jan 24 16:33:33 2018(r328335)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/uefisign/uefisign.c
==
--- head/usr.sbin/uefisign/uefisign.c   Wed Jan 24 15:16:17 2018
(r328334)
+++ head/usr.sbin/uefisign/uefisign.c   Wed Jan 24 16:33:33 2018
(r328335)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/uefisign/uefisign.h
==
--- head/usr.sbin/uefisign/uefisign.h   Wed Jan 24 15:16:17 2018
(r328334)
+++ head/usr.sbin/uefisign/uefisign.h   Wed Jan 24 16:33:33 2018
(r328335)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328337 - head/usr.sbin/ctld

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:37:29 2018
New Revision: 328337
URL: https://svnweb.freebsd.org/changeset/base/328337

Log:
  Add missing SPDX tags for ctld(8).
  
  MFC after:2 weeks

Modified:
  head/usr.sbin/ctld/chap.c
  head/usr.sbin/ctld/isns.c
  head/usr.sbin/ctld/uclparse.c

Modified: head/usr.sbin/ctld/chap.c
==
--- head/usr.sbin/ctld/chap.c   Wed Jan 24 16:34:37 2018(r328336)
+++ head/usr.sbin/ctld/chap.c   Wed Jan 24 16:37:29 2018(r328337)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/ctld/isns.c
==
--- head/usr.sbin/ctld/isns.c   Wed Jan 24 16:34:37 2018(r328336)
+++ head/usr.sbin/ctld/isns.c   Wed Jan 24 16:37:29 2018(r328337)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 Alexander Motin 
  * All rights reserved.
  *

Modified: head/usr.sbin/ctld/uclparse.c
==
--- head/usr.sbin/ctld/uclparse.c   Wed Jan 24 16:34:37 2018
(r328336)
+++ head/usr.sbin/ctld/uclparse.c   Wed Jan 24 16:37:29 2018
(r328337)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2015 iXsystems Inc.
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328338 - head/usr.sbin/autofs

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:39:02 2018
New Revision: 328338
URL: https://svnweb.freebsd.org/changeset/base/328338

Log:
  Add SPDX tags for automount(8) et al.
  
  MFC after:2 weeks

Modified:
  head/usr.sbin/autofs/automount.c
  head/usr.sbin/autofs/automountd.c
  head/usr.sbin/autofs/autounmountd.c
  head/usr.sbin/autofs/common.c
  head/usr.sbin/autofs/common.h
  head/usr.sbin/autofs/defined.c
  head/usr.sbin/autofs/log.c
  head/usr.sbin/autofs/popen.c
  head/usr.sbin/autofs/token.l

Modified: head/usr.sbin/autofs/automount.c
==
--- head/usr.sbin/autofs/automount.cWed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/automount.cWed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/automountd.c
==
--- head/usr.sbin/autofs/automountd.c   Wed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/automountd.c   Wed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/autounmountd.c
==
--- head/usr.sbin/autofs/autounmountd.c Wed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/autounmountd.c Wed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/common.c
==
--- head/usr.sbin/autofs/common.c   Wed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/common.c   Wed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/common.h
==
--- head/usr.sbin/autofs/common.h   Wed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/common.h   Wed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/defined.c
==
--- head/usr.sbin/autofs/defined.c  Wed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/defined.c  Wed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/log.c
==
--- head/usr.sbin/autofs/log.c  Wed Jan 24 16:37:29 2018(r328337)
+++ head/usr.sbin/autofs/log.c  Wed Jan 24 16:39:02 2018(r328338)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2012 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/usr.sbin/autofs/popen.c
==
--- head/usr.sbin/autofs/popen.cWed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/popen.cWed Jan 24 16:39:02 2018
(r328338)
@@ -1,4 +1,6 @@
 /*
+ * SPDX-License-Identifier: BSD-3-Clause
+ *
  * Copyright (c) 1988, 1993
  * The Regents of the University of California.  All rights reserved.
  * Copyright (c) 2014 The FreeBSD Foundation

Modified: head/usr.sbin/autofs/token.l
==
--- head/usr.sbin/autofs/token.lWed Jan 24 16:37:29 2018
(r328337)
+++ head/usr.sbin/autofs/token.lWed Jan 24 16:39:02 2018
(r328338)
@@ -1,5 +1,7 @@
 %{
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328339 - head/sys/fs/autofs

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:40:26 2018
New Revision: 328339
URL: https://svnweb.freebsd.org/changeset/base/328339

Log:
  Add SPDX tags to autofs(5).
  
  MFC after:2 weeks

Modified:
  head/sys/fs/autofs/autofs.h
  head/sys/fs/autofs/autofs_ioctl.h
  head/sys/fs/autofs/autofs_vfsops.c
  head/sys/fs/autofs/autofs_vnops.c

Modified: head/sys/fs/autofs/autofs.h
==
--- head/sys/fs/autofs/autofs.h Wed Jan 24 16:39:02 2018(r328338)
+++ head/sys/fs/autofs/autofs.h Wed Jan 24 16:40:26 2018(r328339)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/sys/fs/autofs/autofs_ioctl.h
==
--- head/sys/fs/autofs/autofs_ioctl.h   Wed Jan 24 16:39:02 2018
(r328338)
+++ head/sys/fs/autofs/autofs_ioctl.h   Wed Jan 24 16:40:26 2018
(r328339)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2013 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/sys/fs/autofs/autofs_vfsops.c
==
--- head/sys/fs/autofs/autofs_vfsops.c  Wed Jan 24 16:39:02 2018
(r328338)
+++ head/sys/fs/autofs/autofs_vfsops.c  Wed Jan 24 16:40:26 2018
(r328339)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/sys/fs/autofs/autofs_vnops.c
==
--- head/sys/fs/autofs/autofs_vnops.c   Wed Jan 24 16:39:02 2018
(r328338)
+++ head/sys/fs/autofs/autofs_vnops.c   Wed Jan 24 16:40:26 2018
(r328339)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328340 - in head/sys: fs/ext2fs ufs/ffs ufs/ufs

[Pedro F. Giffuni]

Author: pfg
Date: Wed Jan 24 16:44:57 2018
New Revision: 328340
URL: https://svnweb.freebsd.org/changeset/base/328340

Log:
  Revert r327781,  r328093, r328056:
  ufs|ext2fs: Revert uses of mallocarray(9).
  
  These aren't really useful: drop them.
  Variable unsigning will be brought again later.

Modified:
  head/sys/fs/ext2fs/ext2_lookup.c
  head/sys/fs/ext2fs/ext2_vfsops.c
  head/sys/ufs/ffs/ffs_snapshot.c
  head/sys/ufs/ffs/ffs_softdep.c
  head/sys/ufs/ufs/ufs_dirhash.c
  head/sys/ufs/ufs/ufs_vnops.c

Modified: head/sys/fs/ext2fs/ext2_lookup.c
==
--- head/sys/fs/ext2fs/ext2_lookup.cWed Jan 24 16:40:26 2018
(r328339)
+++ head/sys/fs/ext2fs/ext2_lookup.cWed Jan 24 16:44:57 2018
(r328340)
@@ -145,9 +145,9 @@ ext2_readdir(struct vop_readdir_args *ap)
off_t offset, startoffset;
size_t readcnt, skipcnt;
ssize_t startresid;
+   int ncookies;
int DIRBLKSIZ = VTOI(ap->a_vp)->i_e2fs->e2fs_bsize;
int error;
-   u_int ncookies;
 
if (uio->uio_offset < 0)
return (EINVAL);
@@ -160,8 +160,7 @@ ext2_readdir(struct vop_readdir_args *ap)
ncookies = ip->i_size - uio->uio_offset;
ncookies = ncookies / (offsetof(struct ext2fs_direct_2,
e2d_namlen) + 4) + 1;
-   cookies = mallocarray(ncookies, sizeof(*cookies), M_TEMP,
-   M_WAITOK);
+   cookies = malloc(ncookies * sizeof(*cookies), M_TEMP, M_WAITOK);
*ap->a_ncookies = ncookies;
*ap->a_cookies = cookies;
} else {

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==
--- head/sys/fs/ext2fs/ext2_vfsops.cWed Jan 24 16:40:26 2018
(r328339)
+++ head/sys/fs/ext2fs/ext2_vfsops.cWed Jan 24 16:44:57 2018
(r328340)
@@ -400,9 +400,9 @@ compute_sb_data(struct vnode *devvp, struct ext2fs *es
fs->e2fs_bsize / sizeof(struct ext2_gd));
}
fs->e2fs_gdbcount = howmany(fs->e2fs_gcount, e2fs_descpb);
-   fs->e2fs_gd = mallocarray(e2fs_gdbcount_alloc, fs->e2fs_bsize,
+   fs->e2fs_gd = malloc(e2fs_gdbcount_alloc * fs->e2fs_bsize,
M_EXT2MNT, M_WAITOK | M_ZERO);
-   fs->e2fs_contigdirs = mallocarray(fs->e2fs_gcount,
+   fs->e2fs_contigdirs = malloc(fs->e2fs_gcount *
sizeof(*fs->e2fs_contigdirs), M_EXT2MNT, M_WAITOK | M_ZERO);
 
/*
@@ -683,8 +683,7 @@ ext2_mountfs(struct vnode *devvp, struct mount *mp)
for (i = 0; i < ump->um_e2fs->e2fs_gcount; i++, sump++) {
*lp++ = ump->um_e2fs->e2fs_contigsumsize;
sump->cs_init = 0;
-   sump->cs_sum = mallocarray(
-   ump->um_e2fs->e2fs_contigsumsize + 1,
+   sump->cs_sum = malloc((ump->um_e2fs->e2fs_contigsumsize 
+ 1) *
sizeof(int32_t), M_EXT2MNT, M_WAITOK | M_ZERO);
}
}

Modified: head/sys/ufs/ffs/ffs_snapshot.c
==
--- head/sys/ufs/ffs/ffs_snapshot.c Wed Jan 24 16:40:26 2018
(r328339)
+++ head/sys/ufs/ffs/ffs_snapshot.c Wed Jan 24 16:44:57 2018
(r328340)
@@ -648,7 +648,7 @@ loop:
 * keep us out of deadlock until the full one is ready.
 */
if (xp == NULL) {
-   snapblklist = mallocarray(snaplistsize, sizeof(daddr_t),
+   snapblklist = malloc(snaplistsize * sizeof(daddr_t),
M_UFSMNT, M_WAITOK);
blkp = &snapblklist[1];
*blkp++ = lblkno(fs, fs->fs_sblockloc);
@@ -729,7 +729,7 @@ out1:
/*
 * Allocate space for the full list of preallocated snapshot blocks.
 */
-   snapblklist = mallocarray(snaplistsize, sizeof(daddr_t),
+   snapblklist = malloc(snaplistsize * sizeof(daddr_t),
M_UFSMNT, M_WAITOK);
ip->i_snapblklist = &snapblklist[1];
/*

Modified: head/sys/ufs/ffs/ffs_softdep.c
==
--- head/sys/ufs/ffs/ffs_softdep.c  Wed Jan 24 16:40:26 2018
(r328339)
+++ head/sys/ufs/ffs/ffs_softdep.c  Wed Jan 24 16:44:57 2018
(r328340)
@@ -2466,8 +2466,7 @@ softdep_mount(devvp, mp, fs, cred)
struct ufsmount *ump;
struct cg *cgp;
struct buf *bp;
-   u_int cyl, i;
-   int error;
+   int i, error, cyl;
 
sdp = malloc(sizeof(struct mount_softdeps), M_MOUNTDATA,
M_WAITOK | M_ZERO);
@@ -2501,7 +2500,7 @@ softdep_mount(devvp, mp, fs, cred)
ump->bmsafemap_hashtbl = hashinit(1024, M_BMSAFEMAP,
&ump->bmsafemap_hash_size);
i = 1 << (ffs(desiredvnodes / 10) - 1);
-   ump->indir_hashtbl = mallocar

svn commit: r328341 - head/sys/dev/iscsi

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 16:58:26 2018
New Revision: 328341
URL: https://svnweb.freebsd.org/changeset/base/328341

Log:
  Add SPDX tags to iscsi(4).
  
  MFC after:2 weeks

Modified:
  head/sys/dev/iscsi/icl_conn_if.m
  head/sys/dev/iscsi/icl_soft.c
  head/sys/dev/iscsi/icl_wrappers.h

Modified: head/sys/dev/iscsi/icl_conn_if.m
==
--- head/sys/dev/iscsi/icl_conn_if.mWed Jan 24 16:44:57 2018
(r328340)
+++ head/sys/dev/iscsi/icl_conn_if.mWed Jan 24 16:58:26 2018
(r328341)
@@ -1,4 +1,6 @@
 #-
+# SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+#
 # Copyright (c) 2014 The FreeBSD Foundation
 # All rights reserved.
 #

Modified: head/sys/dev/iscsi/icl_soft.c
==
--- head/sys/dev/iscsi/icl_soft.c   Wed Jan 24 16:44:57 2018
(r328340)
+++ head/sys/dev/iscsi/icl_soft.c   Wed Jan 24 16:58:26 2018
(r328341)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2012 The FreeBSD Foundation
  * All rights reserved.
  *

Modified: head/sys/dev/iscsi/icl_wrappers.h
==
--- head/sys/dev/iscsi/icl_wrappers.h   Wed Jan 24 16:44:57 2018
(r328340)
+++ head/sys/dev/iscsi/icl_wrappers.h   Wed Jan 24 16:58:26 2018
(r328341)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2014 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328342 - in head/sys: amd64/linux dev/usb/storage

[Edward Tomasz Napierala]

Author: trasz
Date: Wed Jan 24 17:04:01 2018
New Revision: 328342
URL: https://svnweb.freebsd.org/changeset/base/328342

Log:
  Add SPDX identifiers to linux_ptrace.c and cfumass.c.
  
  MFC after:2 weeks

Modified:
  head/sys/amd64/linux/linux_ptrace.c
  head/sys/dev/usb/storage/cfumass.c

Modified: head/sys/amd64/linux/linux_ptrace.c
==
--- head/sys/amd64/linux/linux_ptrace.c Wed Jan 24 16:58:26 2018
(r328341)
+++ head/sys/amd64/linux/linux_ptrace.c Wed Jan 24 17:04:01 2018
(r328342)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2017 Edward Tomasz Napierala 
  * All rights reserved.
  *

Modified: head/sys/dev/usb/storage/cfumass.c
==
--- head/sys/dev/usb/storage/cfumass.c  Wed Jan 24 16:58:26 2018
(r328341)
+++ head/sys/dev/usb/storage/cfumass.c  Wed Jan 24 17:04:01 2018
(r328342)
@@ -1,4 +1,6 @@
 /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
  * Copyright (c) 2016 The FreeBSD Foundation
  * All rights reserved.
  *
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328343 - head/usr.bin/time

[Alan Somers]

Author: asomers
Date: Wed Jan 24 17:12:34 2018
New Revision: 328343
URL: https://svnweb.freebsd.org/changeset/base/328343

Log:
  time(1): use clock_gettime(2) instead of gettimeofday(2)
  
  This is a prerequisite to adding support for the monotonic clock
  
  Reviewed by:  ken, imp
  MFC after:3 weeks
  Sponsored by: Spectra Logic Corp
  Differential Revision:https://reviews.freebsd.org/D14030

Modified:
  head/usr.bin/time/time.c

Modified: head/usr.bin/time/time.c
==
--- head/usr.bin/time/time.cWed Jan 24 17:04:01 2018(r328342)
+++ head/usr.bin/time/time.cWed Jan 24 17:12:34 2018(r328343)
@@ -58,18 +58,19 @@ static const char rcsid[] =
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static int getstathz(void);
 static void humantime(FILE *, long, long);
-static void showtime(FILE *, struct timeval *, struct timeval *,
+static void showtime(FILE *, struct timespec *, struct timespec *,
 struct rusage *);
 static void siginfo(int);
 static void usage(void);
 
 static sig_atomic_t siginfo_recvd;
 static char decimal_point;
-static struct timeval before_tv;
+static struct timespec before_ts;
 static int hflag, pflag;
 
 int
@@ -80,7 +81,7 @@ main(int argc, char **argv)
pid_t pid;
struct rlimit rl;
struct rusage ru;
-   struct timeval after;
+   struct timespec after;
char *ofn = NULL;
FILE *out = stderr;
 
@@ -120,7 +121,8 @@ main(int argc, char **argv)
setvbuf(out, (char *)NULL, _IONBF, (size_t)0);
}
 
-   (void)gettimeofday(&before_tv, NULL);
+   if (clock_gettime(CLOCK_REALTIME, &before_ts))
+   err(1, "clock_gettime");
switch(pid = fork()) {
case -1:/* error */
err(1, "time");
@@ -139,16 +141,18 @@ main(int argc, char **argv)
while (wait4(pid, &status, 0, &ru) != pid) {
if (siginfo_recvd) {
siginfo_recvd = 0;
-   (void)gettimeofday(&after, NULL);
+   if (clock_gettime(CLOCK_REALTIME, &after))
+   err(1, "clock_gettime");
getrusage(RUSAGE_CHILDREN, &ru);
-   showtime(stdout, &before_tv, &after, &ru);
+   showtime(stdout, &before_ts, &after, &ru);
}
}
-   (void)gettimeofday(&after, NULL);
+   if (clock_gettime(CLOCK_REALTIME, &after))
+   err(1, "clock_gettime");
if ( ! WIFEXITED(status))
warnx("command terminated abnormally");
exitonsig = WIFSIGNALED(status) ? WTERMSIG(status) : 0;
-   showtime(out, &before_tv, &after, &ru);
+   showtime(out, &before_ts, &after, &ru);
if (lflag) {
int hz = getstathz();
u_long ticks;
@@ -237,7 +241,7 @@ getstathz(void)
 }
 
 static void
-humantime(FILE *out, long sec, long usec)
+humantime(FILE *out, long sec, long centisec)
 {
long days, hrs, mins;
 
@@ -255,18 +259,18 @@ humantime(FILE *out, long sec, long usec)
fprintf(out, "%ldh", hrs);
if (mins)
fprintf(out, "%ldm", mins);
-   fprintf(out, "%ld%c%02lds", sec, decimal_point, usec);
+   fprintf(out, "%ld%c%02lds", sec, decimal_point, centisec);
 }
 
 static void
-showtime(FILE *out, struct timeval *before, struct timeval *after,
+showtime(FILE *out, struct timespec *before, struct timespec *after,
 struct rusage *ru)
 {
 
after->tv_sec -= before->tv_sec;
-   after->tv_usec -= before->tv_usec;
-   if (after->tv_usec < 0)
-   after->tv_sec--, after->tv_usec += 100;
+   after->tv_nsec -= before->tv_nsec;
+   if (after->tv_nsec < 0)
+   after->tv_sec--, after->tv_nsec += 10;
 
if (pflag) {
/* POSIX wants output that must look like
@@ -274,7 +278,7 @@ showtime(FILE *out, struct timeval *before, struct tim
at least two digits after the radix. */
fprintf(out, "real %jd%c%02ld\n",
(intmax_t)after->tv_sec, decimal_point,
-   after->tv_usec/1);
+   after->tv_nsec/1000);
fprintf(out, "user %jd%c%02ld\n",
(intmax_t)ru->ru_utime.tv_sec, decimal_point,
ru->ru_utime.tv_usec/1);
@@ -282,7 +286,7 @@ showtime(FILE *out, struct timeval *before, struct tim
(intmax_t)ru->ru_stime.tv_sec, decimal_point,
ru->ru_stime.tv_usec/1);
} else if (hflag) {
-   humantime(out, after->tv_sec, after->tv_usec/1);
+   humantime(out, after->tv_sec, after->tv_nsec/1000);
fprintf(out, " real\t");
humantime(out, ru->ru_utime.tv_sec, ru->ru_utime.tv_usec/1)

Re: svn commit: r328342 - in head/sys: amd64/linux dev/usb/storage

[Pedro Giffuni]

On 01/24/18 12:04, Edward Tomasz Napierala wrote:

Author: trasz
Date: Wed Jan 24 17:04:01 2018
New Revision: 328342
URL: https://svnweb.freebsd.org/changeset/base/328342

Log:
   Add SPDX identifiers to linux_ptrace.c and cfumass.c.
   
   MFC after:	2 weeks



Thanks for all these!

For the record ... I am not considering MFCing SPDX tags.
There is just too much to be done for current to worry about stable at 
all ... but I won't stop anyone from doing so ;).


Pedro.


Modified:
   head/sys/amd64/linux/linux_ptrace.c
   head/sys/dev/usb/storage/cfumass.c

Modified: head/sys/amd64/linux/linux_ptrace.c
==
--- head/sys/amd64/linux/linux_ptrace.c Wed Jan 24 16:58:26 2018
(r328341)
+++ head/sys/amd64/linux/linux_ptrace.c Wed Jan 24 17:04:01 2018
(r328342)
@@ -1,4 +1,6 @@
  /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
   * Copyright (c) 2017 Edward Tomasz Napierala 
   * All rights reserved.
   *

Modified: head/sys/dev/usb/storage/cfumass.c
==
--- head/sys/dev/usb/storage/cfumass.c  Wed Jan 24 16:58:26 2018
(r328341)
+++ head/sys/dev/usb/storage/cfumass.c  Wed Jan 24 17:04:01 2018
(r328342)
@@ -1,4 +1,6 @@
  /*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
   * Copyright (c) 2016 The FreeBSD Foundation
   * All rights reserved.
   *



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328342 - in head/sys: amd64/linux dev/usb/storage

[Edward Tomasz Napierala]

On 0124T1216, Pedro Giffuni wrote:
> 
> On 01/24/18 12:04, Edward Tomasz Napierala wrote:
> > Author: trasz
> > Date: Wed Jan 24 17:04:01 2018
> > New Revision: 328342
> > URL: https://svnweb.freebsd.org/changeset/base/328342
> >
> > Log:
> >Add SPDX identifiers to linux_ptrace.c and cfumass.c.
> >
> >MFC after:   2 weeks
> 
> 
> Thanks for all these!

Thanks for all the rest :-)

> For the record ... I am not considering MFCing SPDX tags.
> There is just too much to be done for current to worry about stable at 
> all ... but I won't stop anyone from doing so ;).

I'm not sure if I will MFC them either, to be honest.  For me this field
is added automatically, via ~/.subversion/config setting - it's better
to get a reminder that I can ignore than to forget.

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Pedro Giffuni]

On 01/24/18 03:50, Bruce Evans wrote:

On Tue, 23 Jan 2018, Pedro Giffuni wrote:


On 23/01/2018 14:08, Conrad Meyer wrote:

Hi Pedro,

On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni  
wrote:

Author: pfg
Date: Sun Jan 21 15:42:36 2018
New Revision: 328218
URL: https://svnweb.freebsd.org/changeset/base/328218

Log:
   Revert r327828, r327949, r327953, r328016-r328026, r328041:
   Uses of mallocarray(9).

   The use of mallocarray(9) has rocketed the required swap to 
build FreeBSD.
   This is likely caused by the allocation size attributes which 
put extra pressure

   on the compiler.

I'm confused about this change.  Wouldn't it be better to remove the
annotation/attributes from mallocarray() than to remove the protection
against overflow?


It would be better to remove mallocarray().

I am fine with that: Its probably better to stop the bug now before it 
propagates.

This said, several drivers, including drm2, have #defines to do the same.

Not in my opinion: it would be better to detect such overflows at 
compile time (or through a static analyzer) than to have late 
notification though panics. The blind use of mallocarray(9) is 
probably a mistake also: we shouldn't use it unless there is some 
real risk of overflow.


Excellent!  There is no real risk of overflow except in cases that are
broken anyway.  The overflow checks in mallocarray() are insufficient 
even

for detecting overflow and don't detect most broken cases when they are
sufficient.  This leaves no cases where even non-blind use of it does any
good.


   (If the compiler is fixed in the future to not use
excessive memory with these attributes, they can be conditionalized on
compiler version, of course.)


All in all, the compiler is not provably wrong: it's just using more 
swap space, which is rather inconvenient for small platforms but not 
necessarily wrong.


I don't know why the compiler uses more swap space, but the general 
brokenness

of mallocarray() is obvious.

Callers must somehow ensure that the allocation size is not too large.
Too large is closer to 64K than SIZE_MAX for most allocations. Some
bloatware allocates a few MB, but malloc() is rarely used for that.
vmstat -m has been broken to not show a summary of sizes at the end,
but on freefall now it seems to shows a maximum malloc() size of 64K,
and vmstat -z confirms this by not showing any malloc() bucket sizes
larger than 64K, and in fact kern_malloc.c only has statically allocatd
bucket sizes up to 64K.  (vmstat -z never had a summary at the end to
break).  Much larger allocations are mostly done using k*alloc(), and
slightly larger allocations are usually done using UMA.  zfs does
zillions of allocations using UMA, and the largest one seems to be 16M.

If the caller doesn't check, this gives a Denial of Service security hole
if the size and count are controlled by userland.  If the size and count
are controlled by the kernel, then the overflow check is not very useful.
It is less useful than most KASSERT()s which are left out of production
kernels.  The caller must keep its allocation sizes not much more than
64K, and once it does that it is unlikely to make them larger than 
SIZE_MAX

sometimes.

The overflow checks in mallocarray have many type errors so they don't
always work:

X Modified: head/share/man/man9/malloc.9
X 
==

X --- head/share/man/man9/malloc.9    Sun Jan  7 10:29:15 2018 (r327673)
X +++ head/share/man/man9/malloc.9    Sun Jan  7 13:21:01 2018 (r327674)
X @@ -45,6 +45,8 @@
X  .In sys/malloc.h
X  .Ft void *
X  .Fn malloc "unsigned long size" "struct malloc_type *type" "int flags"
X +.Ft void *
X +.Fn mallocarray "size_t nmemb" "size_t size" "struct malloc_type 
*type" "int flags"


One type error is already obvious.  malloc(9)'s arg doesn't have type 
size_t
like malloc(3)'s arg.  The arg type is u_long in the kernel. 
mallocarray()'s

types are inconsistent with this.



This would be relatively easy to "fix", at least so that the types match 
malloc(9).

The rest is likely more painful, if it has solution at all.


size_t happens to have the same representation as u_long on all supported
arches, so this doesn't cause many problems now.  On 32-bit arches, 
size_t

hs type u_int.  u_int has smaller rank than u_long, so compilers could
reasonably complain that converting a u_long to a size_t is a downcast.
They shouldn't complain that converting a size_t to a u_long to pass it
to malloc() is an upcast, so there is no problem in typical sloppy code
that uses size_t for sizes and passes these to malloc().  More careful
ciode would use u_long for size to pass to malloc() and compiler's might
complain about downcasting to pass to mallocarray() instead.

Otherwise (on exotic machines with size_t larger than u_long), passing
size_t's to malloc() is a bug unless they values have been checked to
be <= ULONG_MAX, and mallocarrary()'s inconsistent API expands this bug.

X Modifie

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 7:44 AM, Warner Losh  wrote:
> I agree completely. It doesn't do what you think it is doing, for all the
> reasons that Bruce outlines. We thought it was a bad idea when it came up 2
> years ago and nothing has really changed.

I disagree.  I'm not sure what you mean by "it doesn't do what you
think it is doing."  Do you think the manual page is unclear or needs
more detail?  It seems clear to me, but it also does what I think it
does.

Your description of two years ago is inaccurate — you thought it was a
bad idea, and were the most vocal on the mailing list about it, but
that viewpoint was not universally shared.  In a pure headcount vote I
think you were even outvoted, but as the initiative was headed by a
non-committer, it sputtered out.

If Bruce has made some important point or illumination, please
highlight it.  It's buried in the mostly nonsense wall of text
boilerplate he usually includes.

mallocarray serves an important function — a last ditch seatbelt
against overflowing allocations that can trivially replace existing
naive malloc calls containing multiplication.  Trivial heap corruption
is replaced with DoS — a strict improvement.  That is all it does.

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Tue, Jan 23, 2018 at 11:40 AM, Pedro Giffuni  wrote:
> On 23/01/2018 14:08, Conrad Meyer wrote:
>> On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni  wrote:
>>>
>>> Author: pfg
>>> Date: Sun Jan 21 15:42:36 2018
>>> New Revision: 328218
>>
>> I'm confused about this change.  Wouldn't it be better to remove the
>> annotation/attributes from mallocarray() than to remove the protection
>> against overflow?
>
>
> Not in my opinion: it would be better to detect such overflows at compile
> time (or through a static analyzer) than to have late notification though
> panics.

Sure, it would be better, but some situations are only detected at
runtime -- hence mallocarray.  And occasional use of the annotations
on systems with plenty of RAM would keep the source tree free of
compiler-detectable overflows, which I suspect are incredibly
uncommon.

> The blind use of mallocarray(9) is probably a mistake also: we
> shouldn't use it unless there is some real risk of overflow.

I'm not sure I follow that.

>>(If the compiler is fixed in the future to not use
>> excessive memory with these attributes, they can be conditionalized on
>> compiler version, of course.)
>
> All in all, the compiler is not provably wrong: it's just using more swap
> space, which is rather inconvenient for small platforms but not necessarily
> wrong.

Seems wrong if it's a noticeable amount.  Maybe we could flip the
annotations on or off with a low-ram build knob or something like
that.

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328344 - head/tests/sys/kern

[John Baldwin]

Author: jhb
Date: Wed Jan 24 17:46:20 2018
New Revision: 328344
URL: https://svnweb.freebsd.org/changeset/base/328344

Log:
  Mark the unused argument to continue_thread() as such.
  
  clang in HEAD and 11 does not warn about this, but clang in 10 does.

Modified:
  head/tests/sys/kern/ptrace_test.c

Modified: head/tests/sys/kern/ptrace_test.c
==
--- head/tests/sys/kern/ptrace_test.c   Wed Jan 24 17:12:34 2018
(r328343)
+++ head/tests/sys/kern/ptrace_test.c   Wed Jan 24 17:46:20 2018
(r328344)
@@ -3474,7 +3474,7 @@ ATF_TC_BODY(ptrace__PT_STEP_with_signal, tc)
  * that restarting doesn't retrigger the breakpoint.
  */
 static void *
-continue_thread(void *arg)
+continue_thread(void *arg __unused)
 {
breakpoint();
return (NULL);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328345 - head/sys/arm/freescale/imx

[Ian Lepore]

Author: ian
Date: Wed Jan 24 17:52:06 2018
New Revision: 328345
URL: https://svnweb.freebsd.org/changeset/base/328345

Log:
  Reformat indentation to match other imx5/6 register definition headers, and
  tweak some comments.  No functional changes.

Modified:
  head/sys/arm/freescale/imx/imx_wdogreg.h

Modified: head/sys/arm/freescale/imx/imx_wdogreg.h
==
--- head/sys/arm/freescale/imx/imx_wdogreg.hWed Jan 24 17:46:20 2018
(r328344)
+++ head/sys/arm/freescale/imx/imx_wdogreg.hWed Jan 24 17:52:06 2018
(r328345)
@@ -34,31 +34,31 @@
 #defineWDOG_CLK_FREQ   32768
 
 #defineWDOG_CR_REG 0x00/* Control Register */
-#defineWDOG_CR_WT_MASK 0xff00  /* Count of 0.5 sec */
-#defineWDOG_CR_WT_SHIFT8
-#defineWDOG_CR_WDW (1 << 7) /* Suspend WDog */
-#defineWDOG_CR_WDA (1 << 5) /* Don't touch 
ipp_wdog */
-#defineWDOG_CR_SRS (1 << 4) /* Don't touch 
sys_reset */
-#defineWDOG_CR_WDT (1 << 3) /* Assert ipp_wdog on 
tout */
-#defineWDOG_CR_WDE (1 << 2) /* WDog Enable */
-#defineWDOG_CR_WDBG(1 << 1) /* Suspend when DBG 
mode */
-#defineWDOG_CR_WDZST   (1 << 0) /* Suspend when LP 
mode */
+#define  WDOG_CR_WT_MASK 0xff00/* Count; 0.5 sec units */
+#define  WDOG_CR_WT_SHIFT8
+#define  WDOG_CR_WDW (1u << 7) /* Suspend when in WAIT 
mode */
+#define  WDOG_CR_WDA (1u << 5) /* Don't assert ext reset */
+#define  WDOG_CR_SRS (1u << 4) /* Don't assert soft reset 
*/
+#define  WDOG_CR_WDT (1u << 3) /* Assert ext reset on 
timeout */
+#define  WDOG_CR_WDE (1u << 2) /* Watchdog Enable */
+#define  WDOG_CR_WDBG(1u << 1) /* Suspend when DBG mode */
+#define  WDOG_CR_WDZST   (1u << 0) /* Suspend when LP mode */
 
 #defineWDOG_SR_REG 0x02/* Service Register */
-#defineWDOG_SR_STEP1   0x
-#defineWDOG_SR_STEP2   0x
+#define  WDOG_SR_STEP1   0x
+#define  WDOG_SR_STEP2   0x
 
 #defineWDOG_RSR_REG0x04/* Reset Status Register */
-#defineWDOG_RSR_POR(1 << 4) /* Due to Power-On 
Reset */
-#defineWDOG_RSR_TOUT   (1 << 1) /* Due WDog timeout 
reset */
-#defineWDOG_RSR_SFTW   (1 << 0) /* Due Soft reset */
+#define  WDOG_RSR_POR(1u << 4) /* Due to Power-On Reset */
+#define  WDOG_RSR_TOUT   (1u << 1) /* Due WDog timeout reset */
+#define  WDOG_RSR_SFTW   (1u << 0) /* Due Soft reset */
 
 #defineWDOG_ICR_REG0x06/* Interrupt Control Register */
-#defineWDOG_ICR_WIE(1 << 15) /* Enable Interrupt */
-#defineWDOG_ICR_WTIS   (1 << 14) /* Interrupt has 
occurred */
-#defineWDOG_ICR_WTCT_MASK  0x00ff
-#defineWDOG_ICR_WTCT_SHIFT 0   /* Interrupt hold time 
*/
+#define  WDOG_ICR_WIE(1u << 15) /* Enable Interrupt */
+#define  WDOG_ICR_WTIS   (1u << 14) /* Interrupt has occurred 
*/
+#define  WDOG_ICR_WTCT_MASK  0x00ff /* Interrupt lead time in 
0.5s */
+#define  WDOG_ICR_WTCT_SHIFT 0  /* units before reset 
occurs */
 
 #defineWDOG_MCR_REG0x08/* Miscellaneous Control Register */
-#defineWDOG_MCR_PDE(1 << 0)
+#define  WDOG_MCR_PDE(1u << 0)  /* Power-down enable */
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328346 - in head/sys: fs/ext2fs ufs/ffs ufs/ufs

[Pedro F. Giffuni]

Author: pfg
Date: Wed Jan 24 17:58:48 2018
New Revision: 328346
URL: https://svnweb.freebsd.org/changeset/base/328346

Log:
  ext2fs|ufs:Unsign some values related to allocation.
  
  When allocating memory through malloc(9), we always expect the amount of
  memory requested to be unsigned as a negative value would either stand for
  an error or an overflow.
  Unsign some values, found when considering the use of mallocarray(9), to
  avoid unnecessary casting. Also consider that indexes should be of
  at least the same size/type as the upper limit they pretend to index.
  
  MFC after:2 weeks

Modified:
  head/sys/fs/ext2fs/ext2_lookup.c
  head/sys/ufs/ffs/ffs_softdep.c
  head/sys/ufs/ufs/ufs_dirhash.c
  head/sys/ufs/ufs/ufs_vnops.c

Modified: head/sys/fs/ext2fs/ext2_lookup.c
==
--- head/sys/fs/ext2fs/ext2_lookup.cWed Jan 24 17:52:06 2018
(r328345)
+++ head/sys/fs/ext2fs/ext2_lookup.cWed Jan 24 17:58:48 2018
(r328346)
@@ -145,9 +145,9 @@ ext2_readdir(struct vop_readdir_args *ap)
off_t offset, startoffset;
size_t readcnt, skipcnt;
ssize_t startresid;
-   int ncookies;
int DIRBLKSIZ = VTOI(ap->a_vp)->i_e2fs->e2fs_bsize;
int error;
+   u_int ncookies;
 
if (uio->uio_offset < 0)
return (EINVAL);

Modified: head/sys/ufs/ffs/ffs_softdep.c
==
--- head/sys/ufs/ffs/ffs_softdep.c  Wed Jan 24 17:52:06 2018
(r328345)
+++ head/sys/ufs/ffs/ffs_softdep.c  Wed Jan 24 17:58:48 2018
(r328346)
@@ -2466,7 +2466,8 @@ softdep_mount(devvp, mp, fs, cred)
struct ufsmount *ump;
struct cg *cgp;
struct buf *bp;
-   int i, error, cyl;
+   u_int cyl, i;
+   int error;
 
sdp = malloc(sizeof(struct mount_softdeps), M_MOUNTDATA,
M_WAITOK | M_ZERO);

Modified: head/sys/ufs/ufs/ufs_dirhash.c
==
--- head/sys/ufs/ufs/ufs_dirhash.c  Wed Jan 24 17:52:06 2018
(r328345)
+++ head/sys/ufs/ufs/ufs_dirhash.c  Wed Jan 24 17:58:48 2018
(r328346)
@@ -349,7 +349,8 @@ ufsdirhash_build(struct inode *ip)
struct direct *ep;
struct vnode *vp;
doff_t bmask, pos;
-   int dirblocks, i, j, memreqd, nblocks, narrays, nslots, slot;
+   u_int dirblocks, i, narrays, nblocks, nslots;
+   int j, memreqd, slot;
 
/* Take care of a decreased sysctl value. */
while (ufs_dirhashmem > ufs_dirhashmaxmem) {

Modified: head/sys/ufs/ufs/ufs_vnops.c
==
--- head/sys/ufs/ufs/ufs_vnops.cWed Jan 24 17:52:06 2018
(r328345)
+++ head/sys/ufs/ufs/ufs_vnops.cWed Jan 24 17:58:48 2018
(r328346)
@@ -2170,7 +2170,7 @@ ufs_readdir(ap)
off_t offset, startoffset;
size_t readcnt, skipcnt;
ssize_t startresid;
-   int ncookies;
+   u_int ncookies;
int error;
 
if (uio->uio_offset < 0)
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Ian Lepore]

On Wed, 2018-01-24 at 09:34 -0800, Conrad Meyer wrote:
> On Wed, Jan 24, 2018 at 7:44 AM, Warner Losh  wrote:
> > 
> > I agree completely. It doesn't do what you think it is doing, for all the
> > reasons that Bruce outlines. We thought it was a bad idea when it came up 2
> > years ago and nothing has really changed.
> I disagree.  I'm not sure what you mean by "it doesn't do what you
> think it is doing."  Do you think the manual page is unclear or needs
> more detail?  It seems clear to me, but it also does what I think it
> does.
> 
> Your description of two years ago is inaccurate — you thought it was a
> bad idea, and were the most vocal on the mailing list about it, but
> that viewpoint was not universally shared.  In a pure headcount vote I
> think you were even outvoted, but as the initiative was headed by a
> non-committer, it sputtered out.
> 
> If Bruce has made some important point or illumination, please
> highlight it.  It's buried in the mostly nonsense wall of text
> boilerplate he usually includes.
> 
> mallocarray serves an important function — a last ditch seatbelt
> against overflowing allocations that can trivially replace existing
> naive malloc calls containing multiplication.  Trivial heap corruption
> is replaced with DoS — a strict improvement.  That is all it does.
> 

The fact that you trivially dismiss the detailed explanation of why the
function isn't useful without even reading it means that the only real
option is to also trivially dismiss anything you have to say on the
subject.

I agree that Bruce's long missives are often full of non-actionable
information, and I usually don't bother to read them unless I intend to
respond on the subject being discussed.  But when the arguments
presented in them are on-point, however tediously expressed, then you
litterally CANNOT dismiss them and expect anyone to take you seriously.

And for what it's worth, I also come down on the side of removing and
not using mallocarray(), but I freely admit it's more of a gut-reaction 
thing for me than something I arrived at by careful analysis (so my
opinion is also pretty easily dismissable -- after 40 years at this, I
tend to trust my gut, but I don't expect anyone else to).

-- Ian

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Wed, Jan 24, 2018 at 10:34 AM, Conrad Meyer  wrote:

> On Wed, Jan 24, 2018 at 7:44 AM, Warner Losh  wrote:
> > I agree completely. It doesn't do what you think it is doing, for all the
> > reasons that Bruce outlines. We thought it was a bad idea when it came
> up 2
> > years ago and nothing has really changed.
>
> I disagree.  I'm not sure what you mean by "it doesn't do what you
> think it is doing."  Do you think the manual page is unclear or needs
> more detail?  It seems clear to me, but it also does what I think it
> does.
>

It changes the fundamental 'can't fail' allocations into 'maybe panic'
allocations, which was my big objection.


> Your description of two years ago is inaccurate — you thought it was a
> bad idea, and were the most vocal on the mailing list about it, but
> that viewpoint was not universally shared.  In a pure headcount vote I
> think you were even outvoted, but as the initiative was headed by a
> non-committer, it sputtered out.
>

I don't recall that happening. But regardless, mallocarray, as implemented
today, is useless.


> If Bruce has made some important point or illumination, please
> highlight it.  It's buried in the mostly nonsense wall of text
> boilerplate he usually includes.
>

Let's start with his point about u_long vs size_t causing problems:

void*malloc(unsigned long size, struct malloc_type *type, int flags)
vs
void*mallocarray(size_t nmemb, size_t size, struct malloc_type *type,

Since size_t is long long on i386, for example, this can result in
undetected overflows. Such inattention to detail makes me extremely uneasy
about the rest of the code.

mallocarray serves an important function — a last ditch seatbelt
> against overflowing allocations that can trivially replace existing
> naive malloc calls containing multiplication.  Trivial heap corruption
> is replaced with DoS — a strict improvement.  That is all it does.
>

It's an important function, but it's so poorly implement we should try
again. It is not safe nor wise to use it blindly, which was bde's larger
point.

#define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 8 / 2))
static inline bool
WOULD_OVERFLOW(size_t nmemb, size_t size)
{

return ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
nmemb > 0 && __SIZE_T_MAX / nmemb < size);
}

So if I pass in 1GB and 10, this will tell me it won't overflow because of
size_t can handle 10GB, but u_long can't. On amd64, it won't, but on i386
it will since long is 32 bits leading to issues in this code:

void *
mallocarray(size_t nmemb, size_t size, struct malloc_type *type, int flags)
{

if (WOULD_OVERFLOW(nmemb, size))
panic("mallocarray: %zu * %zu overflowed", nmemb, size);

return (malloc(size * nmemb, type, flags));
}

since malloc takes u_long, size * nmemb would overflow yielding bad results.

Many places that use mallocarray likely should use WOULD_OVERFLOW instead
to do proper error handling, assuming it is fixed to match the actual
malloc interface. This is especially true in the NO_WAIT cases.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328347 - head

[Bryan Drewery]

Author: bdrewery
Date: Wed Jan 24 18:08:37 2018
New Revision: 328347
URL: https://svnweb.freebsd.org/changeset/base/328347

Log:
  X_COMPILER_* may not be defined.
  
  Sponsored by: Dell EMC

Modified:
  head/Makefile.inc1
  head/Makefile.libcompat

Modified: head/Makefile.inc1
==
--- head/Makefile.inc1  Wed Jan 24 17:58:48 2018(r328346)
+++ head/Makefile.inc1  Wed Jan 24 18:08:37 2018(r328347)
@@ -637,7 +637,8 @@ XCFLAGS+=   -isystem ${WORLDTMP}/usr/include -L${WORLDTM
 # combined with --sysroot.
 XCFLAGS+=  -B${WORLDTMP}/usr/lib
 # Force using libc++ for external GCC.
-.if ${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800
+.if defined(X_COMPILER_TYPE) && \
+${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800
 XCXXFLAGS+=-isystem ${WORLDTMP}/usr/include/c++/v1 -std=c++11 \
-nostdinc++
 .endif
@@ -2880,7 +2881,8 @@ CD2CFLAGS+=   -isystem ${XDDESTDIR}/usr/include 
-L${XDDE
 # combined with --sysroot.
 CD2CFLAGS+=-B${XDDESTDIR}/usr/lib
 # Force using libc++ for external GCC.
-.if ${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800
+.if defined(X_COMPILER_TYPE) && \
+${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800
 CD2CXXFLAGS+=  -isystem ${XDDESTDIR}/usr/include/c++/v1 -std=c++11 \
-nostdinc++
 .endif

Modified: head/Makefile.libcompat
==
--- head/Makefile.libcompat Wed Jan 24 17:58:48 2018(r328346)
+++ head/Makefile.libcompat Wed Jan 24 18:08:37 2018(r328347)
@@ -108,7 +108,8 @@ LIBCOMPATCFLAGS+=   -B${LIBCOMPATTMP}/usr/lib${libcompat
 # sysroot path which --sysroot does not actually do for headers.
 LIBCOMPATCFLAGS+=  -isystem ${LIBCOMPATTMP}/usr/include
 # Force using libc++ for external GCC.
-.if ${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800 && \
+.if defined(X_COMPILER_TYPE) && \
+${X_COMPILER_TYPE} == gcc && ${X_COMPILER_VERSION} >= 40800 && \
 (${MK_CLANG_BOOTSTRAP} == "no" && ${MK_GCC_BOOTSTRAP} == "no")
 LIBCOMPATCXXFLAGS+=-isystem ${LIBCOMPATTMP}/usr/include/c++/v1 -std=c++11 \
-nostdinc++
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328348 - head

[Bryan Drewery]

Author: bdrewery
Date: Wed Jan 24 18:09:44 2018
New Revision: 328348
URL: https://svnweb.freebsd.org/changeset/base/328348

Log:
  test-system-compiler: Display X_ variants for compiler/linker.
  
  Sponsored by: Dell EMC

Modified:
  head/Makefile.inc1

Modified: head/Makefile.inc1
==
--- head/Makefile.inc1  Wed Jan 24 18:08:37 2018(r328347)
+++ head/Makefile.inc1  Wed Jan 24 18:09:44 2018(r328348)
@@ -144,7 +144,10 @@ TEST_SYSTEM_COMPILER_VARS= \
WANT_COMPILER_FREEBSD_VERSION WANT_COMPILER_FREEBSD_VERSION_FILE \
CC COMPILER_TYPE COMPILER_FEATURES COMPILER_VERSION \
COMPILER_FREEBSD_VERSION \
-   LINKER_TYPE LINKER_FEATURES LINKER_VERSION
+   X_COMPILER_TYPE X_COMPILER_FEATURES X_COMPILER_VERSION \
+   X_COMPILER_FREEBSD_VERSION \
+   LINKER_TYPE LINKER_FEATURES LINKER_VERSION \
+   X_LINKER_TYPE X_LINKER_FEATURES X_LINKER_VERSION
 test-system-compiler: .PHONY
 .for v in ${TEST_SYSTEM_COMPILER_VARS}
${_+_}@printf "%-35s= %s\n" "${v}" "${${v}}"
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328349 - head/sys/arm/freescale/imx

[Ian Lepore]

Author: ian
Date: Wed Jan 24 18:10:11 2018
New Revision: 328349
URL: https://svnweb.freebsd.org/changeset/base/328349

Log:
  Make the trivial imx_soc_family() function an inline in imx_machdep.h.
  
  The imx_machdep.c file is on the fast path to non-existance and this would
  be the only thing left in it after some watchdog changes are completed.

Modified:
  head/sys/arm/freescale/imx/imx_machdep.c
  head/sys/arm/freescale/imx/imx_machdep.h

Modified: head/sys/arm/freescale/imx/imx_machdep.c
==
--- head/sys/arm/freescale/imx/imx_machdep.cWed Jan 24 18:09:44 2018
(r328348)
+++ head/sys/arm/freescale/imx/imx_machdep.cWed Jan 24 18:10:11 2018
(r328349)
@@ -105,10 +105,3 @@ imx_wdog_init_last_reset(vm_offset_t wdsr_phys)
}
 }
 
-u_int
-imx_soc_family(void)
-{
-   return (imx_soc_type() >> IMXSOC_FAMSHIFT);
-}
-
-

Modified: head/sys/arm/freescale/imx/imx_machdep.h
==
--- head/sys/arm/freescale/imx/imx_machdep.hWed Jan 24 18:09:44 2018
(r328348)
+++ head/sys/arm/freescale/imx/imx_machdep.hWed Jan 24 18:10:11 2018
(r328349)
@@ -61,7 +61,12 @@ void imx_wdog_init_last_reset(vm_offset_t _wdsr_phys);
 #defineIMXSOC_FAMSHIFT 28
 
 u_int imx_soc_type(void);
-u_int imx_soc_family(void);
+
+static inline u_int
+imx_soc_family(void)
+{
+   return (imx_soc_type() >> IMXSOC_FAMSHIFT);
+}
 
 #endif
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Pedro Giffuni]

On 01/24/18 12:37, Conrad Meyer wrote:

On Tue, Jan 23, 2018 at 11:40 AM, Pedro Giffuni  wrote:

On 23/01/2018 14:08, Conrad Meyer wrote:

On Sun, Jan 21, 2018 at 7:42 AM, Pedro F. Giffuni  wrote:

Author: pfg
Date: Sun Jan 21 15:42:36 2018
New Revision: 328218

I'm confused about this change.  Wouldn't it be better to remove the
annotation/attributes from mallocarray() than to remove the protection
against overflow?


Not in my opinion: it would be better to detect such overflows at compile
time (or through a static analyzer) than to have late notification though
panics.

Sure, it would be better, but some situations are only detected at
runtime -- hence mallocarray.  And occasional use of the annotations
on systems with plenty of RAM would keep the source tree free of
compiler-detectable overflows, which I suspect are incredibly
uncommon.


I think the runtime error cases are way more uncommon, specially if the 
checks are unnecessary.
In any case I collected the patch with malloc--> mallocarray changes in 
PR 225197.

Maybe their are useful with fuzzing.


The blind use of mallocarray(9) is probably a mistake also: we
shouldn't use it unless there is some real risk of overflow.

I'm not sure I follow that.


You normally don't get "tainted" values in the kernel. Most of the 
allocations in question were variables with very small number multiplied 
by the size of an int. As Bruce mentioned, we don't do big allocations 
with malloc(9), at least not the size required to get an unsigned number 
overflow. In such cases checking for an overflow is a complete waste of 
time. And then there is also the bug of mallocarray(9) size types not 
matching malloc(9).



(If the compiler is fixed in the future to not use
excessive memory with these attributes, they can be conditionalized on
compiler version, of course.)

All in all, the compiler is not provably wrong: it's just using more swap
space, which is rather inconvenient for small platforms but not necessarily
wrong.

Seems wrong if it's a noticeable amount.  Maybe we could flip the
annotations on or off with a low-ram build knob or something like
that.


There is actually no proof that this was related to the attributes. I 
absolutely dislike the idea of disabling the attributes and even more 
the idea of adding complex machinery to the system headers to account 
for such case.


Pedro.

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

Please point out what in Bruce's rant is actually relevant.  Again, I
usually start reading them and get sidetracked in things that are
opinions stated as fact, or outright incorrect.  At which point, I
give up on them.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Konstantin Belousov]

On Wed, Jan 24, 2018 at 11:05:24AM -0700, Warner Losh wrote:
> Let's start with his point about u_long vs size_t causing problems:
> 
> void*malloc(unsigned long size, struct malloc_type *type, int flags)
> vs
> void*mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
> 
> Since size_t is long long on i386, for example, this can result in
> undetected overflows. Such inattention to detail makes me extremely uneasy
> about the rest of the code.

size_t has same representation as u_long on all supported arches.
size_t is 32bit on i386, why could it need to be 64bit on 32bit architecture ?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.

Warner

On Jan 24, 2018 11:20 AM, "Conrad Meyer"  wrote:

> Please point out what in Bruce's rant is actually relevant.  Again, I
> usually start reading them and get sidetracked in things that are
> opinions stated as fact, or outright incorrect.  At which point, I
> give up on them.
>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 10:05 AM, Warner Losh  wrote:
> It changes the fundamental 'can't fail' allocations into 'maybe panic'
> allocations, which was my big objection.

No, it doesn't make any such change.  The only calls that will panic
now would have "succeeded" before but returned a smaller size than the
naive caller thought they were asking for.  This allows an attacker to
dereference beyond the ends of the actually allocated region.

>> Your description of two years ago is inaccurate — you thought it was a
>> bad idea, and were the most vocal on the mailing list about it, but
>> that viewpoint was not universally shared.  In a pure headcount vote I
>> think you were even outvoted, but as the initiative was headed by a
>> non-committer, it sputtered out.
>
>
> I don't recall that happening. But regardless, mallocarray, as implemented
> today, is useless.

Search your email inbox for the mallocarray thread from Feb 2016.  I
don't think it's useless.

> Let's start with his point about u_long vs size_t causing problems:
>
> void*malloc(unsigned long size, struct malloc_type *type, int flags)
> vs
> void*mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
>
> Since size_t is long long on i386, for example,

It is?  Since when?  __size_t is uint32_t on !__LP64__.

> this can result in
> undetected overflows. Such inattention to detail makes me extremely uneasy
> about the rest of the code.

A similar snarky comment can be made here about inattention to detail
when making criticisms.  If __LP64__ is true, long long is the same
size as long anyway.

(malloc() should also use size_t.)

> It's an important function, but it's so poorly implement we should try
> again. It is not safe nor wise to use it blindly, which was bde's larger
> point.

Citation needed?

> #define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 8 / 2))
> static inline bool
> WOULD_OVERFLOW(size_t nmemb, size_t size)
> {
>
> return ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) &&
> nmemb > 0 && __SIZE_T_MAX / nmemb < size);
> }
>
> So if I pass in 1GB and 10, this will tell me it won't overflow because of
> size_t can handle 10GB, but u_long can't.

This whole argument hinges upon incorrect assumption that size_t is
larger than u_long.

> ... (deleted bogus argument)

> Many places that use mallocarray likely should use WOULD_OVERFLOW instead to
> do proper error handling, assuming it is fixed to match the actual malloc
> interface. This is especially true in the NO_WAIT cases.

I disagree.  They should be doing a much more restrictive check
instead.  WOULD_OVERFLOW is really the lowest bar, a seatbelt.  I
agree with Bruce that most callers should instead be checking for
sizes in the dozens of kB range.  Both checks are useful.

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

Bruce didn't get this wrong, you've just misread his (style / opinion)
complaint as an actual bug (which is kind of the whole reason why it's
hard to treat his complaints seriously):

> size_t happens to have the same representation as u_long on all supported 
> arches

So yes, the check works on i386.

Best,
Conrad

On Wed, Jan 24, 2018 at 10:28 AM, Warner Losh  wrote:
> Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.
>
> Warner
>
> On Jan 24, 2018 11:20 AM, "Conrad Meyer"  wrote:
>>
>> Please point out what in Bruce's rant is actually relevant.  Again, I
>> usually start reading them and get sidetracked in things that are
>> opinions stated as fact, or outright incorrect.  At which point, I
>> give up on them.
>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Hans Petter Selasky]

On 01/24/18 19:28, Warner Losh wrote:

Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.


Hi,

If M_WAITOK is specified, then sleep forever and print a message.
Else return NULL ?

--HPS
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Jan 24, 2018 11:33 AM, "Conrad Meyer"  wrote:

Bruce didn't get this wrong, you've just misread his (style / opinion)
complaint as an actual bug (which is kind of the whole reason why it's
hard to treat his complaints seriously):

> size_t happens to have the same representation as u_long on all supported
arches

So yes, the check works on i386.


I confused off_t and size_t, so much of what I said turns out not to be
relevant.

I'd be fine with just fixing the style issue, renaming WOULD_OVERFLOW to
malloc_would_overrflow and using that for most of the NO_WAIT cases as a
precheck

Warner

Warner

Best,
Conrad

On Wed, Jan 24, 2018 at 10:28 AM, Warner Losh  wrote:
> Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.
>
> Warner
>
> On Jan 24, 2018 11:20 AM, "Conrad Meyer"  wrote:
>>
>> Please point out what in Bruce's rant is actually relevant.  Again, I
>> usually start reading them and get sidetracked in things that are
>> opinions stated as fact, or outright incorrect.  At which point, I
>> give up on them.
>>
>
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 10:40 AM, Hans Petter Selasky  wrote:
> On 01/24/18 19:28, Warner Losh wrote:
>>
>> Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.
>
>
> Hi,
>
> If M_WAITOK is specified, then sleep forever and print a message.
> Else return NULL ?

Depends on the machine.  From malloc.9:

> If the multiplication of nmemb and size would cause an integer overflow, the 
> mallocarray() function induces a panic.

64-bit arch does not experience overflow and will sleep until 10GB is
available.  (This is why the caller should do a narrower check.)

32-bit panics, since 10 * 1GB overflows 32-bit size_t (== u_long).

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Pedro Giffuni]

...
On 24/01/2018 13:30, Conrad Meyer wrote:

...
size_t can handle 10GB, but u_long can't.
This whole argument hinges upon incorrect assumption that size_t is
larger than u_long.


Hmm...

Lets just make it "unsigned long" to be consistent with malloc(9) and 
avoid confusion?


Pedro.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328350 - head/sys/netipsec

[Andrey V. Elsukov]

Author: ae
Date: Wed Jan 24 19:06:44 2018
New Revision: 328350
URL: https://svnweb.freebsd.org/changeset/base/328350

Log:
  Merge revision 1.35 from NetBSD:
fix pointer/offset mistakes in handling of IPv4 options
  
  Reported by:  Maxime Villard 
  MFC after:1 week

Modified:
  head/sys/netipsec/xform_ah.c

Modified: head/sys/netipsec/xform_ah.c
==
--- head/sys/netipsec/xform_ah.cWed Jan 24 18:10:11 2018
(r328349)
+++ head/sys/netipsec/xform_ah.cWed Jan 24 19:06:44 2018
(r328350)
@@ -293,7 +293,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
else
ip->ip_off = htons(0);
 
-   ptr = mtod(m, unsigned char *) + sizeof(struct ip);
+   ptr = mtod(m, unsigned char *);
 
/* IPv4 option processing */
for (off = sizeof(struct ip); off < skip;) {
@@ -374,7 +374,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 
/* Zeroize all other options. */
count = ptr[off + 1];
-   bcopy(ipseczeroes, ptr, count);
+   bcopy(ipseczeroes, ptr + off, count);
off += count;
break;
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Pedro Giffuni]

On 24/01/2018 13:40, Hans Petter Selasky wrote:

On 01/24/18 19:28, Warner Losh wrote:

Does mallocarray(10 ,1Gb) panic on i386? It does not. It should.


Hi,

If M_WAITOK is specified, then sleep forever and print a message.
Else return NULL ?



FWIW, I suggested a panic for M_WAITOK and returning NULL for M_NOWAIT, 
but cem didn't like it because it was inconsistent.


I think the current argument is more about the size/trigger than the 
behavior though.


Cheers,
Pedro.

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 10:51 AM, Pedro Giffuni  wrote:
> FWIW, I suggested a panic for M_WAITOK and returning NULL for M_NOWAIT, but
> cem didn't like it because it was inconsistent.
>
> I think the current argument is more about the size/trigger than the
> behavior though.

Yeah.  If an overflow happens in a path, we want to flag it --
regardless of whether that was a M_NOWAIT or M_WAITOK request.

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 10:44 AM, Pedro Giffuni  wrote:
> ...
> On 24/01/2018 13:30, Conrad Meyer wrote:
>>
>> ...
>> size_t can handle 10GB, but u_long can't.
>> This whole argument hinges upon incorrect assumption that size_t is
>> larger than u_long.
>
>
> Hmm...
>
> Lets just make it "unsigned long" to be consistent with malloc(9) and avoid
> confusion?

The opposite, I think -- change malloc(9) and realloc*(9) to size_t to
be more consistent with the C standard and malloc(3).

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Jan 24, 2018 12:24 PM, "Conrad Meyer"  wrote:

On Wed, Jan 24, 2018 at 10:51 AM, Pedro Giffuni  wrote:
> FWIW, I suggested a panic for M_WAITOK and returning NULL for M_NOWAIT,
but
> cem didn't like it because it was inconsistent.
>
> I think the current argument is more about the size/trigger than the
> behavior though.

Yeah.  If an overflow happens in a path, we want to flag it --
regardless of whether that was a M_NOWAIT or M_WAITOK request.


Which is why we should add check overflows for most of the no wait cases.
They should be checked, but not primarily with mallocarray...

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328351 - in head: share/man/man9 sys/kern sys/sys

[Conrad Meyer]

Author: cem
Date: Wed Jan 24 19:37:18 2018
New Revision: 328351
URL: https://svnweb.freebsd.org/changeset/base/328351

Log:
  malloc(9): Change nominal size to size_t to match standard C
  
  No functional change -- size_t matches unsigned long on all platforms.
  
  Reported by:  bde
  Discussed with:   jhb
  Sponsored by: Dell EMC Isilon

Modified:
  head/share/man/man9/malloc.9
  head/sys/kern/kern_malloc.c
  head/sys/sys/malloc.h

Modified: head/share/man/man9/malloc.9
==
--- head/share/man/man9/malloc.9Wed Jan 24 19:06:44 2018
(r328350)
+++ head/share/man/man9/malloc.9Wed Jan 24 19:37:18 2018
(r328351)
@@ -29,7 +29,7 @@
 .\" $NetBSD: malloc.9,v 1.3 1996/11/11 00:05:11 lukem Exp $
 .\" $FreeBSD$
 .\"
-.Dd January 10, 2018
+.Dd January 24, 2018
 .Dt MALLOC 9
 .Os
 .Sh NAME
@@ -44,15 +44,15 @@
 .In sys/types.h
 .In sys/malloc.h
 .Ft void *
-.Fn malloc "unsigned long size" "struct malloc_type *type" "int flags"
+.Fn malloc "size_t size" "struct malloc_type *type" "int flags"
 .Ft void *
 .Fn mallocarray "size_t nmemb" "size_t size" "struct malloc_type *type" "int 
flags"
 .Ft void
 .Fn free "void *addr" "struct malloc_type *type"
 .Ft void *
-.Fn realloc "void *addr" "unsigned long size" "struct malloc_type *type" "int 
flags"
+.Fn realloc "void *addr" "size_t size" "struct malloc_type *type" "int flags"
 .Ft void *
-.Fn reallocf "void *addr" "unsigned long size" "struct malloc_type *type" "int 
flags"
+.Fn reallocf "void *addr" "size_t size" "struct malloc_type *type" "int flags"
 .Fn MALLOC_DECLARE type
 .In sys/param.h
 .In sys/malloc.h

Modified: head/sys/kern/kern_malloc.c
==
--- head/sys/kern/kern_malloc.c Wed Jan 24 19:06:44 2018(r328350)
+++ head/sys/kern/kern_malloc.c Wed Jan 24 19:37:18 2018(r328351)
@@ -518,7 +518,7 @@ malloc_dbg(caddr_t *vap, unsigned long *sizep, struct 
  * the allocation fails.
  */
 void *
-malloc(unsigned long size, struct malloc_type *mtp, int flags)
+malloc(size_t size, struct malloc_type *mtp, int flags)
 {
int indx;
struct malloc_type_internal *mtip;
@@ -567,7 +567,7 @@ malloc(unsigned long size, struct malloc_type *mtp, in
 }
 
 void *
-malloc_domain(unsigned long size, struct malloc_type *mtp, int domain,
+malloc_domain(size_t size, struct malloc_type *mtp, int domain,
 int flags)
 {
int indx;
@@ -754,7 +754,7 @@ free_domain(void *addr, struct malloc_type *mtp)
  * realloc: change the size of a memory block
  */
 void *
-realloc(void *addr, unsigned long size, struct malloc_type *mtp, int flags)
+realloc(void *addr, size_t size, struct malloc_type *mtp, int flags)
 {
uma_slab_t slab;
unsigned long alloc;
@@ -815,7 +815,7 @@ realloc(void *addr, unsigned long size, struct malloc_
  * reallocf: same as realloc() but free memory on failure.
  */
 void *
-reallocf(void *addr, unsigned long size, struct malloc_type *mtp, int flags)
+reallocf(void *addr, size_t size, struct malloc_type *mtp, int flags)
 {
void *mem;
 

Modified: head/sys/sys/malloc.h
==
--- head/sys/sys/malloc.h   Wed Jan 24 19:06:44 2018(r328350)
+++ head/sys/sys/malloc.h   Wed Jan 24 19:37:18 2018(r328351)
@@ -181,11 +181,10 @@ void  *contigmalloc_domain(unsigned long size, struct 
m
__malloc_like __result_use_check __alloc_size(1) __alloc_align(6);
 void   free(void *addr, struct malloc_type *type);
 void   free_domain(void *addr, struct malloc_type *type);
-void   *malloc(unsigned long size, struct malloc_type *type, int flags)
-   __malloc_like __result_use_check __alloc_size(1);
-void   *malloc_domain(unsigned long size, struct malloc_type *type,
-   int domain, int flags)
-   __malloc_like __result_use_check __alloc_size(1);
+void   *malloc(size_t size, struct malloc_type *type, int flags) __malloc_like
+   __result_use_check __alloc_size(1);
+void   *malloc_domain(size_t size, struct malloc_type *type, int domain,
+   int flags) __malloc_like __result_use_check __alloc_size(1);
 void   *mallocarray(size_t nmemb, size_t size, struct malloc_type *type,
int flags) __malloc_like __result_use_check
__alloc_size2(1, 2);
@@ -195,10 +194,10 @@ void  malloc_type_allocated(struct malloc_type *type, 
u
 void   malloc_type_freed(struct malloc_type *type, unsigned long size);
 void   malloc_type_list(malloc_type_list_func_t *, void *);
 void   malloc_uninit(void *);
-void   *realloc(void *addr, unsigned long size, struct malloc_type *type,
-   int flags) __result_use_check __alloc_size(2);
-void   *reallocf(void *addr, unsigned long size, struct malloc_type *type,
-   int flags) __result_use_check __alloc_size(2);
+void   *realloc(void *addr, size_t si

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 11:27 AM, Warner Losh  wrote:
>
> Which is why we should add check overflows for most of the no wait cases.
> They should be checked, but not primarily with mallocarray...

I don't understand what the distinction is here.  Can you help me
understand why the overflow check should be lifted from mallocarray
into the caller for no wait cases?  Or is that not what you're
suggesting?

Thanks,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328352 - head/sys/netipsec

[Andrey V. Elsukov]

Author: ae
Date: Wed Jan 24 19:48:25 2018
New Revision: 328352
URL: https://svnweb.freebsd.org/changeset/base/328352

Log:
  Adopt revision 1.76 and 1.77 from NetBSD:
Fix a vulnerability in IPsec-IPv6-AH, that allows an attacker to remotely
crash the kernel with a single packet.
  
In this loop we need to increment 'ad' by two, because the length field
of the option header does not count the size of the option header itself.
  
If the length is zero, then 'count' is incremented by zero, and there's
an infinite loop. Beyond that, this code was written with the assumption
that since the IPv6 packet already went through the generic IPv6 option
parser, several fields are guaranteed to be valid; but this assumption
does not hold because of the missing '+2', and there's as a result a
triggerable buffer overflow (write zeros after the end of the mbuf,
potentially to the next mbuf in memory since it's a pool).
  
Add the missing '+2', this place will be reinforced in separate commits.
  
  Reported by:  Maxime Villard 
  MFC after:1 week

Modified:
  head/sys/netipsec/xform_ah.c

Modified: head/sys/netipsec/xform_ah.c
==
--- head/sys/netipsec/xform_ah.cWed Jan 24 19:37:18 2018
(r328351)
+++ head/sys/netipsec/xform_ah.cWed Jan 24 19:48:25 2018
(r328352)
@@ -264,7 +264,7 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
 #ifdef INET6
struct ip6_ext *ip6e;
struct ip6_hdr ip6;
-   int alloc, len, ad;
+   int ad, alloc, nxt, noff;
 #endif /* INET6 */
 
switch (proto) {
@@ -447,61 +447,44 @@ ah_massage_headers(struct mbuf **m0, int proto, int sk
} else
break;
 
-   off = ip6.ip6_nxt & 0xff; /* Next header type. */
+   nxt = ip6.ip6_nxt & 0xff; /* Next header type. */
 
-   for (len = 0; len < skip - sizeof(struct ip6_hdr);)
-   switch (off) {
+   for (off = 0; off < skip - sizeof(struct ip6_hdr);)
+   switch (nxt) {
case IPPROTO_HOPOPTS:
case IPPROTO_DSTOPTS:
-   ip6e = (struct ip6_ext *) (ptr + len);
+   ip6e = (struct ip6_ext *)(ptr + off);
+   noff = off + ((ip6e->ip6e_len + 1) << 3);
 
+   /* Sanity check. */
+   if (noff > skip - sizeof(struct ip6_hdr))
+   goto error6;
+
/*
-* Process the mutable/immutable
-* options -- borrows heavily from the
-* KAME code.
+* Zero out mutable options.
 */
-   for (count = len + sizeof(struct ip6_ext);
-count < len + ((ip6e->ip6e_len + 1) << 
3);) {
+   for (count = off + sizeof(struct ip6_ext);
+count < noff;) {
if (ptr[count] == IP6OPT_PAD1) {
count++;
continue; /* Skip padding. */
}
 
-   /* Sanity check. */
-   if (count > len +
-   ((ip6e->ip6e_len + 1) << 3)) {
-   m_freem(m);
+   ad = ptr[count + 1] + 2;
+   if (count + ad > noff)
+   goto error6;
 
-   /* Free, if we allocated. */
-   if (alloc)
-   free(ptr, M_XDATA);
-   return EINVAL;
-   }
-
-   ad = ptr[count + 1];
-
-   /* If mutable option, zeroize. */
if (ptr[count] & IP6OPT_MUTABLE)
-   bcopy(ipseczeroes, ptr + count,
-   ptr[count + 1]);
-
+   memset(ptr + count, 0, ad);
count += ad;
-
-   /* Sanity check. */
-   if (count >
-   skip - sizeof(struct ip6_hdr)) {
-   m

svn commit: r328353 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:04:08 2018
New Revision: 328353
URL: https://svnweb.freebsd.org/changeset/base/328353

Log:
  Always store the IV in the immediate portion of a work request.
  
  Combined authentication-encryption and GCM requests already stored the
  IV in the immediate explicitly.  This extends this behavior to block
  cipher requests to work around a firmware bug.  While here, simplify
  the AEAD and GCM handlers to not include always-true conditions.
  
  Submitted by: Harsh Jain @ Chelsio
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 19:48:25 2018
(r328352)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:04:08 2018
(r328353)
@@ -546,7 +546,7 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
struct wrqe *wr;
struct cryptodesc *crd;
char *dst;
-   u_int iv_loc, kctx_len, key_half, op_type, transhdr_len, wr_len;
+   u_int kctx_len, key_half, op_type, transhdr_len, wr_len;
u_int imm_len;
int dsgl_nsegs, dsgl_len;
int sgl_nsegs, sgl_len;
@@ -564,24 +564,22 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
if (crd->crd_len > MAX_REQUEST_SIZE)
return (EFBIG);
 
-   iv_loc = IV_NOP;
if (crd->crd_flags & CRD_F_ENCRYPT) {
op_type = CHCR_ENCRYPT_OP;
if (crd->crd_flags & CRD_F_IV_EXPLICIT)
memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
else
arc4rand(iv, s->blkcipher.iv_len, 0);
-   iv_loc = IV_IMMEDIATE;
if ((crd->crd_flags & CRD_F_IV_PRESENT) == 0)
crypto_copyback(crp->crp_flags, crp->crp_buf,
crd->crd_inject, s->blkcipher.iv_len, iv);
} else {
op_type = CHCR_DECRYPT_OP;
-   if (crd->crd_flags & CRD_F_IV_EXPLICIT) {
+   if (crd->crd_flags & CRD_F_IV_EXPLICIT)
memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
-   iv_loc = IV_IMMEDIATE;
-   } else
-   iv_loc = IV_DSGL;
+   else
+   crypto_copydata(crp->crp_flags, crp->crp_buf,
+   crd->crd_inject, s->blkcipher.iv_len, iv);
}
 
sglist_reset(sc->sg_dsgl);
@@ -601,22 +599,11 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
if (ccr_use_imm_data(transhdr_len, crd->crd_len +
s->blkcipher.iv_len)) {
imm_len = crd->crd_len;
-   if (iv_loc == IV_DSGL) {
-   crypto_copydata(crp->crp_flags, crp->crp_buf,
-   crd->crd_inject, s->blkcipher.iv_len, iv);
-   iv_loc = IV_IMMEDIATE;
-   }
sgl_nsegs = 0;
sgl_len = 0;
} else {
imm_len = 0;
sglist_reset(sc->sg_ulptx);
-   if (iv_loc == IV_DSGL) {
-   error = sglist_append_sglist(sc->sg_ulptx, sc->sg_crp,
-   crd->crd_inject, s->blkcipher.iv_len);
-   if (error)
-   return (error);
-   }
error = sglist_append_sglist(sc->sg_ulptx, sc->sg_crp,
crd->crd_skip, crd->crd_len);
if (error)
@@ -625,9 +612,8 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
sgl_len = ccr_ulptx_sgl_len(sgl_nsegs);
}
 
-   wr_len = roundup2(transhdr_len, 16) + roundup2(imm_len, 16) + sgl_len;
-   if (iv_loc == IV_IMMEDIATE)
-   wr_len += s->blkcipher.iv_len;
+   wr_len = roundup2(transhdr_len, 16) + s->blkcipher.iv_len +
+   roundup2(imm_len, 16) + sgl_len;
wr = alloc_wrqe(wr_len, sc->txq);
if (wr == NULL) {
sc->stats_wr_nomem++;
@@ -637,7 +623,7 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
memset(crwr, 0, wr_len);
 
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len, 0,
-   iv_loc, crp);
+   IV_IMMEDIATE, crp);
 
/* XXX: Hardcodes SGE loopback channel of 0. */
crwr->sec_cpl.op_ivinsrtofst = htobe32(
@@ -700,10 +686,8 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
dst = (char *)(crwr + 1) + kctx_len;
ccr_write_phys_dsgl(sc, dst, dsgl_nsegs);
dst += sizeof(struct cpl_rx_phys_dsgl) + dsgl_len;
-   if (iv_loc == IV_IMMEDIATE) {
-   memcpy(dst, iv, s->blkcipher.iv_len);
-   dst += s->blkcipher.iv_len;
-   }
+   memcpy(dst, iv, s->blkcipher.iv_len);
+   dst += s->blkcipher.iv_len;
if (imm_len != 0)
  

svn commit: r328354 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:06:02 2018
New Revision: 328354
URL: https://svnweb.freebsd.org/changeset/base/328354

Log:
  Always set the IV location to IV_NOP.
  
  The firmware ignores this field in the FW_CRYPTO_LOOKASIDE_WR work
  request.
  
  Submitted by: Harsh Jain @ Chelsio
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:04:08 2018
(r328353)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:06:02 2018
(r328354)
@@ -365,7 +365,7 @@ ccr_use_imm_data(u_int transhdr_len, u_int input_len)
 static void
 ccr_populate_wreq(struct ccr_softc *sc, struct chcr_wr *crwr, u_int kctx_len,
 u_int wr_len, uint32_t sid, u_int imm_len, u_int sgl_len, u_int hash_size,
-u_int iv_loc, struct cryptop *crp)
+struct cryptop *crp)
 {
u_int cctx_size;
 
@@ -383,7 +383,7 @@ ccr_populate_wreq(struct ccr_softc *sc, struct chcr_wr
V_FW_CRYPTO_LOOKASIDE_WR_RX_CHID(sc->tx_channel_id) |
V_FW_CRYPTO_LOOKASIDE_WR_LCB(0) |
V_FW_CRYPTO_LOOKASIDE_WR_PHASH(0) |
-   V_FW_CRYPTO_LOOKASIDE_WR_IV(iv_loc) |
+   V_FW_CRYPTO_LOOKASIDE_WR_IV(IV_NOP) |
V_FW_CRYPTO_LOOKASIDE_WR_FQIDX(0) |
V_FW_CRYPTO_LOOKASIDE_WR_TX_CH(0) |
V_FW_CRYPTO_LOOKASIDE_WR_RX_Q_ID(sc->rxq->iq.abs_id));
@@ -467,7 +467,7 @@ ccr_hmac(struct ccr_softc *sc, uint32_t sid, struct cc
memset(crwr, 0, wr_len);
 
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len,
-   hash_size_in_response, IV_NOP, crp);
+   hash_size_in_response, crp);
 
/* XXX: Hardcodes SGE loopback channel of 0. */
crwr->sec_cpl.op_ivinsrtofst = htobe32(
@@ -623,7 +623,7 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
memset(crwr, 0, wr_len);
 
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len, 0,
-   IV_IMMEDIATE, crp);
+   crp);
 
/* XXX: Hardcodes SGE loopback channel of 0. */
crwr->sec_cpl.op_ivinsrtofst = htobe32(
@@ -929,8 +929,7 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
memset(crwr, 0, wr_len);
 
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len,
-   op_type == CHCR_DECRYPT_OP ? hash_size_in_response : 0,
-   IV_IMMEDIATE, crp);
+   op_type == CHCR_DECRYPT_OP ? hash_size_in_response : 0, crp);
 
/* XXX: Hardcodes SGE loopback channel of 0. */
crwr->sec_cpl.op_ivinsrtofst = htobe32(
@@ -1227,7 +1226,7 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
memset(crwr, 0, wr_len);
 
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len,
-   0, IV_IMMEDIATE, crp);
+   0, crp);
 
/* XXX: Hardcodes SGE loopback channel of 0. */
crwr->sec_cpl.op_ivinsrtofst = htobe32(
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328355 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:08:10 2018
New Revision: 328355
URL: https://svnweb.freebsd.org/changeset/base/328355

Log:
  Reject requests with AAD and IV larger than 511 bytes.
  
  The T6 crypto engine's control messages only support a total AAD
  length (including the prefixed IV) of 511 bytes.  Reject requests with
  large AAD rather than returning incorrect results.
  
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:06:02 2018
(r328354)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:08:10 2018
(r328355)
@@ -111,6 +111,11 @@ __FBSDID("$FreeBSD$");
  */
 
 /*
+ * The crypto engine supports a maximum AAD size of 511 bytes.
+ */
+#defineMAX_AAD_LEN 511
+
+/*
  * The documentation for CPL_RX_PHYS_DSGL claims a maximum of 32
  * SG entries.
  */
@@ -760,11 +765,23 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
return (EINVAL);
 
/*
-* AAD is only permitted before the cipher/plain text, not
-* after.
+* Compute the length of the AAD (data covered by the
+* authentication descriptor but not the encryption
+* descriptor).  To simplify the logic, AAD is only permitted
+* before the cipher/plain text, not after.  This is true of
+* all currently-generated requests.
 */
if (crda->crd_len + crda->crd_skip > crde->crd_len + crde->crd_skip)
return (EINVAL);
+   if (crda->crd_skip < crde->crd_skip) {
+   if (crda->crd_skip + crda->crd_len > crde->crd_skip)
+   aad_len = (crde->crd_skip - crda->crd_skip);
+   else
+   aad_len = crda->crd_len;
+   } else
+   aad_len = 0;
+   if (aad_len + s->blkcipher.iv_len > MAX_AAD_LEN)
+   return (EINVAL);
 
axf = s->hmac.auth_hash;
hash_size_in_response = s->hmac.hash_len;
@@ -836,13 +853,6 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
 * cipher/plain text.  For decryption requests the hash is
 * appended after the cipher text.
 */
-   if (crda->crd_skip < crde->crd_skip) {
-   if (crda->crd_skip + crda->crd_len > crde->crd_skip)
-   aad_len = (crde->crd_skip - crda->crd_skip);
-   else
-   aad_len = crda->crd_len;
-   } else
-   aad_len = 0;
input_len = aad_len + crde->crd_len;
 
/*
@@ -1080,6 +1090,9 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
 * after.
 */
if (crda->crd_len + crda->crd_skip > crde->crd_len + crde->crd_skip)
+   return (EINVAL);
+
+   if (crda->crd_len + AES_BLOCK_LEN > MAX_AAD_LEN)
return (EINVAL);
 
hash_size_in_response = s->gmac.hash_len;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328356 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:11:00 2018
New Revision: 328356
URL: https://svnweb.freebsd.org/changeset/base/328356

Log:
  Don't discard AAD and IV output data for AEAD requests.
  
  The T6 can hang when processing certain AEAD requests if the request
  sets a flag asking the crypto engine to discard the input IV and AAD
  rather than copying them into the output buffer.  The existing driver
  always discards the IV and AAD as we do not need it.  As a workaround,
  allocate a single "dummy" buffer when the ccr driver attaches and
  change all AEAD requests to write the IV and AAD to this scratch
  buffer.  The contents of the scratch buffer are never used (similar to
  "bogus_page"), and it is ok for multiple in-flight requests to share
  this dummy buffer.
  
  Submitted by: Harsh Jain @ Chelsio (original version)
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:08:10 2018
(r328355)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:11:00 2018
(r328356)
@@ -190,6 +190,13 @@ struct ccr_softc {
struct sglist *sg_ulptx;
struct sglist *sg_dsgl;
 
+   /*
+* Pre-allocate a dummy output buffer for the IV and AAD for
+* AEAD requests.
+*/
+   char *iv_aad_buf;
+   struct sglist *sg_iv_aad;
+
/* Statistics. */
uint64_t stats_blkcipher_encrypt;
uint64_t stats_blkcipher_decrypt;
@@ -814,15 +821,25 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
 * The output buffer consists of the cipher text followed by
 * the hash when encrypting.  For decryption it only contains
 * the plain text.
+*
+* Due to a firmware bug, the output buffer must include a
+* dummy output buffer for the IV and AAD prior to the real
+* output buffer.
 */
if (op_type == CHCR_ENCRYPT_OP) {
-   if (crde->crd_len + hash_size_in_response > MAX_REQUEST_SIZE)
+   if (s->blkcipher.iv_len + aad_len + crde->crd_len +
+   hash_size_in_response > MAX_REQUEST_SIZE)
return (EFBIG);
} else {
-   if (crde->crd_len > MAX_REQUEST_SIZE)
+   if (s->blkcipher.iv_len + aad_len + crde->crd_len >
+   MAX_REQUEST_SIZE)
return (EFBIG);
}
sglist_reset(sc->sg_dsgl);
+   error = sglist_append_sglist(sc->sg_dsgl, sc->sg_iv_aad, 0,
+   s->blkcipher.iv_len + aad_len);
+   if (error)
+   return (error);
error = sglist_append_sglist(sc->sg_dsgl, sc->sg_crp, crde->crd_skip,
crde->crd_len);
if (error)
@@ -977,7 +994,7 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
crwr->sec_cpl.ivgen_hdrlen = htobe32(
V_SCMD_IV_GEN_CTRL(0) |
V_SCMD_MORE_FRAGS(0) | V_SCMD_LAST_FRAG(0) | V_SCMD_MAC_ONLY(0) |
-   V_SCMD_AADIVDROP(1) | V_SCMD_HDR_LEN(dsgl_len));
+   V_SCMD_AADIVDROP(0) | V_SCMD_HDR_LEN(dsgl_len));
 
crwr->key_ctx.ctx_hdr = s->blkcipher.key_ctx_hdr;
switch (crde->crd_alg) {
@@ -1143,15 +1160,24 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
 * The output buffer consists of the cipher text followed by
 * the tag when encrypting.  For decryption it only contains
 * the plain text.
+*
+* Due to a firmware bug, the output buffer must include a
+* dummy output buffer for the IV and AAD prior to the real
+* output buffer.
 */
if (op_type == CHCR_ENCRYPT_OP) {
-   if (crde->crd_len + hash_size_in_response > MAX_REQUEST_SIZE)
+   if (iv_len + crda->crd_len + crde->crd_len +
+   hash_size_in_response > MAX_REQUEST_SIZE)
return (EFBIG);
} else {
-   if (crde->crd_len > MAX_REQUEST_SIZE)
+   if (iv_len + crda->crd_len + crde->crd_len > MAX_REQUEST_SIZE)
return (EFBIG);
}
sglist_reset(sc->sg_dsgl);
+   error = sglist_append_sglist(sc->sg_dsgl, sc->sg_iv_aad, 0, iv_len +
+   crda->crd_len);
+   if (error)
+   return (error);
error = sglist_append_sglist(sc->sg_dsgl, sc->sg_crp, crde->crd_skip,
crde->crd_len);
if (error)
@@ -1287,7 +1313,7 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
crwr->sec_cpl.ivgen_hdrlen = htobe32(
V_SCMD_IV_GEN_CTRL(0) |
V_SCMD_MORE_FRAGS(0) | V_SCMD_LAST_FRAG(0) | V_SCMD_MAC_ONLY(0) |
-   V_SCMD_AADIVDROP(1) | V_SCMD_HDR_LEN(dsgl_len));
+   V_SCMD_AADIVDROP(0) | V_SCMD_HDR_LEN(dsgl_len));
 
crwr->key_ctx.ctx_hdr = s->blkcipher.key_ctx_hdr;

svn commit: r328357 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:12:00 2018
New Revision: 328357
URL: https://svnweb.freebsd.org/changeset/base/328357

Log:
  Fail crypto requests when the resulting work request is too large.
  
  Most crypto requests will not trigger this condition, but a request
  with a highly-fragmented data buffer (and a resulting "large" S/G
  list) could trigger it.
  
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:11:00 2018
(r328356)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:12:00 2018
(r328357)
@@ -470,6 +470,8 @@ ccr_hmac(struct ccr_softc *sc, uint32_t sid, struct cc
}
 
wr_len = roundup2(transhdr_len, 16) + roundup2(imm_len, 16) + sgl_len;
+   if (wr_len > SGE_MAX_WR_LEN)
+   return (EFBIG);
wr = alloc_wrqe(wr_len, sc->txq);
if (wr == NULL) {
sc->stats_wr_nomem++;
@@ -626,6 +628,8 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
 
wr_len = roundup2(transhdr_len, 16) + s->blkcipher.iv_len +
roundup2(imm_len, 16) + sgl_len;
+   if (wr_len > SGE_MAX_WR_LEN)
+   return (EFBIG);
wr = alloc_wrqe(wr_len, sc->txq);
if (wr == NULL) {
sc->stats_wr_nomem++;
@@ -947,6 +951,8 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
 
wr_len = roundup2(transhdr_len, 16) + s->blkcipher.iv_len +
roundup2(imm_len, 16) + sgl_len;
+   if (wr_len > SGE_MAX_WR_LEN)
+   return (EFBIG);
wr = alloc_wrqe(wr_len, sc->txq);
if (wr == NULL) {
sc->stats_wr_nomem++;
@@ -1256,6 +1262,8 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
 
wr_len = roundup2(transhdr_len, 16) + iv_len + roundup2(imm_len, 16) +
sgl_len;
+   if (wr_len > SGE_MAX_WR_LEN)
+   return (EFBIG);
wr = alloc_wrqe(wr_len, sc->txq);
if (wr == NULL) {
sc->stats_wr_nomem++;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328358 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:13:07 2018
New Revision: 328358
URL: https://svnweb.freebsd.org/changeset/base/328358

Log:
  Clamp DSGL entries to a length of 2KB.
  
  This works around an issue in the T6 that can result in DMA engine
  stalls if an error occurs while processing a DSGL entry with a length
  larger than 2KB.
  
  Submitted by: Harsh Jain @ Chelsio
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:12:00 2018
(r328357)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:13:07 2018
(r328358)
@@ -116,11 +116,13 @@ __FBSDID("$FreeBSD$");
 #defineMAX_AAD_LEN 511
 
 /*
- * The documentation for CPL_RX_PHYS_DSGL claims a maximum of 32
- * SG entries.
+ * The documentation for CPL_RX_PHYS_DSGL claims a maximum of 32 SG
+ * entries.  While the CPL includes a 16-bit length field, the T6 can
+ * sometimes hang if an error occurs while processing a request with a
+ * single DSGL entry larger than 2k.
  */
 #defineMAX_RX_PHYS_DSGL_SGE32
-#defineDSGL_SGE_MAXLEN 65535
+#defineDSGL_SGE_MAXLEN 2048
 
 /*
  * The adapter only supports requests with a total input or output
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328359 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:14:57 2018
New Revision: 328359
URL: https://svnweb.freebsd.org/changeset/base/328359

Log:
  Expand the software fallback for GCM to cover more cases.
  
  - Extend ccr_gcm_soft() to handle requests with a non-empty payload.
While here, switch to allocating the GMAC context instead of placing
it on the stack since it is over 1KB in size.
  - Allow ccr_gcm() to return a special error value (EMSGSIZE) which
triggers a fallback to ccr_gcm_soft().  Move the existing empty
payload check into ccr_gcm() and change a few other cases
(e.g. large AAD) to fallback to software via EMSGSIZE as well.
  - Add a new 'sw_fallback' stat to count the number of requests
processed via the software fallback.
  
  Submitted by: Harsh Jain @ Chelsio (original version)
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:13:07 2018
(r328358)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:14:57 2018
(r328359)
@@ -214,6 +214,7 @@ struct ccr_softc {
uint64_t stats_bad_session;
uint64_t stats_sglist_error;
uint64_t stats_process_error;
+   uint64_t stats_sw_fallback;
 };
 
 /*
@@ -,14 +1112,21 @@ ccr_gcm(struct ccr_softc *sc, uint32_t sid, struct ccr
return (EINVAL);
 
/*
+* The crypto engine doesn't handle GCM requests with an empty
+* payload, so handle those in software instead.
+*/
+   if (crde->crd_len == 0)
+   return (EMSGSIZE);
+
+   /*
 * AAD is only permitted before the cipher/plain text, not
 * after.
 */
if (crda->crd_len + crda->crd_skip > crde->crd_len + crde->crd_skip)
-   return (EINVAL);
+   return (EMSGSIZE);
 
if (crda->crd_len + AES_BLOCK_LEN > MAX_AAD_LEN)
-   return (EINVAL);
+   return (EMSGSIZE);
 
hash_size_in_response = s->gmac.hash_len;
 
@@ -1371,19 +1379,55 @@ ccr_gcm_done(struct ccr_softc *sc, struct ccr_session 
 }
 
 /*
- * Handle a GCM request with an empty payload by performing the
- * operation in software.  Derived from swcr_authenc().
+ * Handle a GCM request that is not supported by the crypto engine by
+ * performing the operation in software.  Derived from swcr_authenc().
  */
 static void
 ccr_gcm_soft(struct ccr_session *s, struct cryptop *crp,
 struct cryptodesc *crda, struct cryptodesc *crde)
 {
-   struct aes_gmac_ctx gmac_ctx;
+   struct auth_hash *axf;
+   struct enc_xform *exf;
+   void *auth_ctx;
+   uint8_t *kschedule;
char block[GMAC_BLOCK_LEN];
char digest[GMAC_DIGEST_LEN];
char iv[AES_BLOCK_LEN];
-   int i, len;
+   int error, i, len;
 
+   auth_ctx = NULL;
+   kschedule = NULL;
+
+   /* Initialize the MAC. */
+   switch (s->blkcipher.key_len) {
+   case 16:
+   axf = &auth_hash_nist_gmac_aes_128;
+   break;
+   case 24:
+   axf = &auth_hash_nist_gmac_aes_192;
+   break;
+   case 32:
+   axf = &auth_hash_nist_gmac_aes_256;
+   break;
+   default:
+   error = EINVAL;
+   goto out;
+   }
+   auth_ctx = malloc(axf->ctxsize, M_CCR, M_NOWAIT);
+   if (auth_ctx == NULL) {
+   error = ENOMEM;
+   goto out;
+   }
+   axf->Init(auth_ctx);
+   axf->Setkey(auth_ctx, s->blkcipher.enckey, s->blkcipher.key_len);
+
+   /* Initialize the cipher. */
+   exf = &enc_xform_aes_nist_gcm;
+   error = exf->setkey(&kschedule, s->blkcipher.enckey,
+   s->blkcipher.key_len);
+   if (error)
+   goto out;
+
/*
 * This assumes a 12-byte IV from the crp.  See longer comment
 * above in ccr_gcm() for more details.
@@ -1402,10 +1446,7 @@ ccr_gcm_soft(struct ccr_session *s, struct cryptop *cr
}
*(uint32_t *)&iv[12] = htobe32(1);
 
-   /* Initialize the MAC. */
-   AES_GMAC_Init(&gmac_ctx);
-   AES_GMAC_Setkey(&gmac_ctx, s->blkcipher.enckey, s->blkcipher.key_len);
-   AES_GMAC_Reinit(&gmac_ctx, iv, sizeof(iv));
+   axf->Reinit(auth_ctx, iv, sizeof(iv));
 
/* MAC the AAD. */
for (i = 0; i < crda->crd_len; i += sizeof(block)) {
@@ -1413,29 +1454,70 @@ ccr_gcm_soft(struct ccr_session *s, struct cryptop *cr
crypto_copydata(crp->crp_flags, crp->crp_buf, crda->crd_skip +
i, len, block);
bzero(block + len, sizeof(block) - len);
-   AES_GMAC_Update(&gmac_ctx, block, sizeof(block));
+   axf->Update(auth_ctx, block, sizeof(block));
}
 
+   exf->reinit(kschedule, iv);
+
+   /* Do encryption with MA

svn commit: r328360 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:15:49 2018
New Revision: 328360
URL: https://svnweb.freebsd.org/changeset/base/328360

Log:
  Don't read or generate an IV until all error checking is complete.
  
  In particular, this avoids edge cases where a generated IV might be
  written into the output buffer even though the request is failed with
  an error.
  
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:14:57 2018
(r328359)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:15:49 2018
(r328360)
@@ -581,24 +581,11 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
if (crd->crd_len > MAX_REQUEST_SIZE)
return (EFBIG);
 
-   if (crd->crd_flags & CRD_F_ENCRYPT) {
+   if (crd->crd_flags & CRD_F_ENCRYPT)
op_type = CHCR_ENCRYPT_OP;
-   if (crd->crd_flags & CRD_F_IV_EXPLICIT)
-   memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
-   else
-   arc4rand(iv, s->blkcipher.iv_len, 0);
-   if ((crd->crd_flags & CRD_F_IV_PRESENT) == 0)
-   crypto_copyback(crp->crp_flags, crp->crp_buf,
-   crd->crd_inject, s->blkcipher.iv_len, iv);
-   } else {
+   else
op_type = CHCR_DECRYPT_OP;
-   if (crd->crd_flags & CRD_F_IV_EXPLICIT)
-   memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
-   else
-   crypto_copydata(crp->crp_flags, crp->crp_buf,
-   crd->crd_inject, s->blkcipher.iv_len, iv);
-   }
-
+   
sglist_reset(sc->sg_dsgl);
error = sglist_append_sglist(sc->sg_dsgl, sc->sg_crp, crd->crd_skip,
crd->crd_len);
@@ -641,6 +628,27 @@ ccr_blkcipher(struct ccr_softc *sc, uint32_t sid, stru
crwr = wrtod(wr);
memset(crwr, 0, wr_len);
 
+   /*
+* Read the existing IV from the request or generate a random
+* one if none is provided.  Optionally copy the generated IV
+* into the output buffer if requested.
+*/
+   if (op_type == CHCR_ENCRYPT_OP) {
+   if (crd->crd_flags & CRD_F_IV_EXPLICIT)
+   memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
+   else
+   arc4rand(iv, s->blkcipher.iv_len, 0);
+   if ((crd->crd_flags & CRD_F_IV_PRESENT) == 0)
+   crypto_copyback(crp->crp_flags, crp->crp_buf,
+   crd->crd_inject, s->blkcipher.iv_len, iv);
+   } else {
+   if (crd->crd_flags & CRD_F_IV_EXPLICIT)
+   memcpy(iv, crd->crd_iv, s->blkcipher.iv_len);
+   else
+   crypto_copydata(crp->crp_flags, crp->crp_buf,
+   crd->crd_inject, s->blkcipher.iv_len, iv);
+   }
+
ccr_populate_wreq(sc, crwr, kctx_len, wr_len, sid, imm_len, sgl_len, 0,
crp);
 
@@ -799,30 +807,10 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
 
axf = s->hmac.auth_hash;
hash_size_in_response = s->hmac.hash_len;
-
-   /*
-* The IV is always stored at the start of the buffer even
-* though it may be duplicated in the payload.  The crypto
-* engine doesn't work properly if the IV offset points inside
-* of the AAD region, so a second copy is always required.
-*/
-   if (crde->crd_flags & CRD_F_ENCRYPT) {
+   if (crde->crd_flags & CRD_F_ENCRYPT)
op_type = CHCR_ENCRYPT_OP;
-   if (crde->crd_flags & CRD_F_IV_EXPLICIT)
-   memcpy(iv, crde->crd_iv, s->blkcipher.iv_len);
-   else
-   arc4rand(iv, s->blkcipher.iv_len, 0);
-   if ((crde->crd_flags & CRD_F_IV_PRESENT) == 0)
-   crypto_copyback(crp->crp_flags, crp->crp_buf,
-   crde->crd_inject, s->blkcipher.iv_len, iv);
-   } else {
+   else
op_type = CHCR_DECRYPT_OP;
-   if (crde->crd_flags & CRD_F_IV_EXPLICIT)
-   memcpy(iv, crde->crd_iv, s->blkcipher.iv_len);
-   else
-   crypto_copydata(crp->crp_flags, crp->crp_buf,
-   crde->crd_inject, s->blkcipher.iv_len, iv);
-   }
 
/*
 * The output buffer consists of the cipher text followed by
@@ -876,6 +864,12 @@ ccr_authenc(struct ccr_softc *sc, uint32_t sid, struct
 * The input buffer consists of the IV, any AAD, and then the
 * cipher/plain text.  For decryption requests the hash is
 * appended after the cipher text.
+*
+* The IV is always stored at the start of the input buffer
+   

svn commit: r328361 - head/sys/dev/cxgbe/crypto

[John Baldwin]

Author: jhb
Date: Wed Jan 24 20:16:48 2018
New Revision: 328361
URL: https://svnweb.freebsd.org/changeset/base/328361

Log:
  Store IV in output buffer in GCM software fallback when requested.
  
  Properly honor the lack of the CRD_F_IV_PRESENT flag in the GCM
  software fallback case for encryption requests.
  
  Submitted by: Harsh Jain @ Chelsio
  Sponsored by: Chelsio Communications

Modified:
  head/sys/dev/cxgbe/crypto/t4_crypto.c

Modified: head/sys/dev/cxgbe/crypto/t4_crypto.c
==
--- head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:15:49 2018
(r328360)
+++ head/sys/dev/cxgbe/crypto/t4_crypto.c   Wed Jan 24 20:16:48 2018
(r328361)
@@ -1467,6 +1467,9 @@ ccr_gcm_soft(struct ccr_session *s, struct cryptop *cr
memcpy(iv, crde->crd_iv, 12);
else
arc4rand(iv, 12, 0);
+   if ((crde->crd_flags & CRD_F_IV_PRESENT) == 0)
+   crypto_copyback(crp->crp_flags, crp->crp_buf,
+   crde->crd_inject, 12, iv);
} else {
if (crde->crd_flags & CRD_F_IV_EXPLICIT)
memcpy(iv, crde->crd_iv, 12);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328374 - head/share/man/man8

[Ed Maste]

Author: emaste
Date: Wed Jan 24 21:11:35 2018
New Revision: 328374
URL: https://svnweb.freebsd.org/changeset/base/328374

Log:
  uefi.8: update HISTORY and remove incomplete AUTHORS
  
  - EFI support appeared in 5.0 for ia64
  - arm64 UEFI support added in 11.0
  
  The AUTHORS section included the folks responsible for the bulk of the
  work to bring UEFI support to amd64, but missed those who did the
  original work on ia64, the initial port to i386, the ports to arm64 and
  arm, and have generally maintained and improved general UEFI support
  since then.  It's unwieldly to include everyone and would quickly become
  outdated again anyhow, so just remove the AUTHORS section.
  
  Reviewed by:  manu
  Discussed with:   jhb
  Differential Revision:https://reviews.freebsd.org/D14033

Modified:
  head/share/man/man8/uefi.8

Modified: head/share/man/man8/uefi.8
==
--- head/share/man/man8/uefi.8  Wed Jan 24 20:26:15 2018(r328373)
+++ head/share/man/man8/uefi.8  Wed Jan 24 21:11:35 2018(r328374)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd October 23, 2017
+.Dd January 24, 2018
 .Dt UEFI 8
 .Os
 .Sh NAME
@@ -143,20 +143,13 @@ typical non-default kernel (optional)
 .Xr gpart 8 ,
 .Xr uefisign 8
 .Sh HISTORY
+EFI boot support for the ia64 architecture first appeared in
+.Fx 5.0 .
 .Nm
-boot support first appeared in
-.Fx 10.1 .
-.Sh AUTHORS
-.An -nosplit
-.Nm
-boot support was developed by
-.An Benno Rice Aq Mt be...@freebsd.org ,
-.An \&Ed Maste Aq Mt ema...@freebsd.org ,
-and
-.An Nathan Whitehorn Aq Mt nwhiteh...@freebsd.org .
-The
-.Fx
-Foundation sponsored portions of the work.
+boot support for amd64 first appeared in
+.Fx 10.1
+and for arm64 in
+.Fx 11.0 .
 .Sh CAVEATS
 EFI environment variables are not supported by
 .Xr loader 8
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Wed, Jan 24, 2018 at 12:40 PM, Conrad Meyer  wrote:

> On Wed, Jan 24, 2018 at 11:27 AM, Warner Losh  wrote:
> >
> > Which is why we should add check overflows for most of the no wait cases.
> > They should be checked, but not primarily with mallocarray...
>
> I don't understand what the distinction is here.  Can you help me
> understand why the overflow check should be lifted from mallocarray
> into the caller for no wait cases?  Or is that not what you're
> suggesting?
>

mallocarray should be the last line of defense, not the only line of
defense.

most of the time, it's more correct to say

if (WOULD_OVERFLOW(a,b))
return EINVAL;
ptr = mallocarray(a,b...);

since an error return, assuming it's handled correctly is preferable to a
panic.

I thought this was more true for NOWAIT than for WAITOK cases, but I've
realized it's more true always.

And that's why I have such a problem with mallocarray: it's only useful
when people are lazy and haven't checked, and then it creates a DoS path
for things that don't check. We'll change it now and think we're safe, when
we still have issues, just different issues than before. It may be a
necessary change, but it certainly isn't sufficient.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328375 - head/share/man/man8

[Ed Maste]

Author: emaste
Date: Wed Jan 24 21:20:24 2018
New Revision: 328375
URL: https://svnweb.freebsd.org/changeset/base/328375

Log:
  uefi.8: note that UEFI and EFI are used interchangeably

Modified:
  head/share/man/man8/uefi.8

Modified: head/share/man/man8/uefi.8
==
--- head/share/man/man8/uefi.8  Wed Jan 24 21:11:35 2018(r328374)
+++ head/share/man/man8/uefi.8  Wed Jan 24 21:20:24 2018(r328375)
@@ -39,6 +39,10 @@ to operating systems.
 is a replacement for the legacy BIOS on the i386 and amd64 CPU architectures,
 and is also used on arm, arm64 and ia64.
 .Pp
+The UEFI specification is the successor to the Extensible Firmware Interface
+(EFI) specification.
+The terms are often used interchangeably.
+.Pp
 The
 .Nm
 boot process loads system bootstrap code located in an EFI System Partition
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328376 - head/share/man/man8

[Ed Maste]

Author: emaste
Date: Wed Jan 24 21:26:01 2018
New Revision: 328376
URL: https://svnweb.freebsd.org/changeset/base/328376

Log:
  uefi.8: add .Xr cross references to new efi tools

Modified:
  head/share/man/man8/uefi.8

Modified: head/share/man/man8/uefi.8
==
--- head/share/man/man8/uefi.8  Wed Jan 24 21:20:24 2018(r328375)
+++ head/share/man/man8/uefi.8  Wed Jan 24 21:26:01 2018(r328376)
@@ -144,6 +144,9 @@ typical non-default kernel (optional)
 .Xr boot.config 5 ,
 .Xr msdosfs 5 ,
 .Xr boot 8 ,
+.Xr efibootmgr 8 ,
+.Xr efidp 8 ,
+.Xr efivar 8 ,
 .Xr gpart 8 ,
 .Xr uefisign 8
 .Sh HISTORY
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328377 - in head/sys/dev/etherswitch: arswitch e6000sw infineon ip17x micrel mtkswitch rtl8366 ukswitch

[Michael Zhilin]

Author: mizhka
Date: Wed Jan 24 21:33:18 2018
New Revision: 328377
URL: https://svnweb.freebsd.org/changeset/base/328377

Log:
  [etherswitch] check if_alloc returns NULL
  
  This patch is cosmetic. It checks if allocation of ifnet structure failed.
  It's better to have this check rather than assume positive scenario.
  
  Submitted by: Dmitry Luhtionov 
  Reported by:  Dmitry Luhtionov 

Modified:
  head/sys/dev/etherswitch/arswitch/arswitch.c
  head/sys/dev/etherswitch/e6000sw/e6060sw.c
  head/sys/dev/etherswitch/infineon/adm6996fc.c
  head/sys/dev/etherswitch/ip17x/ip17x.c
  head/sys/dev/etherswitch/micrel/ksz8995ma.c
  head/sys/dev/etherswitch/mtkswitch/mtkswitch.c
  head/sys/dev/etherswitch/rtl8366/rtl8366rb.c
  head/sys/dev/etherswitch/ukswitch/ukswitch.c

Modified: head/sys/dev/etherswitch/arswitch/arswitch.c
==
--- head/sys/dev/etherswitch/arswitch/arswitch.cWed Jan 24 21:26:01 
2018(r328376)
+++ head/sys/dev/etherswitch/arswitch/arswitch.cWed Jan 24 21:33:18 
2018(r328377)
@@ -177,6 +177,12 @@ arswitch_attach_phys(struct arswitch_softc *sc)
snprintf(name, IFNAMSIZ, "%sport", device_get_nameunit(sc->sc_dev));
for (phy = 0; phy < sc->numphys; phy++) {
sc->ifp[phy] = if_alloc(IFT_ETHER);
+   if (sc->ifp[phy] == NULL) {
+   device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
+   err = ENOMEM;
+   break;
+   }
+
sc->ifp[phy]->if_softc = sc;
sc->ifp[phy]->if_flags |= IFF_UP | IFF_BROADCAST |
IFF_DRV_RUNNING | IFF_SIMPLEX;

Modified: head/sys/dev/etherswitch/e6000sw/e6060sw.c
==
--- head/sys/dev/etherswitch/e6000sw/e6060sw.c  Wed Jan 24 21:26:01 2018
(r328376)
+++ head/sys/dev/etherswitch/e6000sw/e6060sw.c  Wed Jan 24 21:33:18 2018
(r328377)
@@ -218,6 +218,12 @@ e6060sw_attach_phys(struct e6060sw_softc *sc)
sc->ifpport[phy] = port;
sc->portphy[port] = phy;
sc->ifp[port] = if_alloc(IFT_ETHER);
+   if (sc->ifp[port] == NULL) {
+   device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
+   err = ENOMEM;
+   break;
+   }
+
sc->ifp[port]->if_softc = sc;
sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
IFF_DRV_RUNNING | IFF_SIMPLEX;

Modified: head/sys/dev/etherswitch/infineon/adm6996fc.c
==
--- head/sys/dev/etherswitch/infineon/adm6996fc.c   Wed Jan 24 21:26:01 
2018(r328376)
+++ head/sys/dev/etherswitch/infineon/adm6996fc.c   Wed Jan 24 21:33:18 
2018(r328377)
@@ -175,6 +175,12 @@ adm6996fc_attach_phys(struct adm6996fc_softc *sc)
sc->ifpport[phy] = port;
sc->portphy[port] = phy;
sc->ifp[port] = if_alloc(IFT_ETHER);
+   if (sc->ifp[port] == NULL) {
+   device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
+   err = ENOMEM;
+   break;
+   }
+
sc->ifp[port]->if_softc = sc;
sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
IFF_DRV_RUNNING | IFF_SIMPLEX;

Modified: head/sys/dev/etherswitch/ip17x/ip17x.c
==
--- head/sys/dev/etherswitch/ip17x/ip17x.c  Wed Jan 24 21:26:01 2018
(r328376)
+++ head/sys/dev/etherswitch/ip17x/ip17x.c  Wed Jan 24 21:33:18 2018
(r328377)
@@ -174,6 +174,12 @@ ip17x_attach_phys(struct ip17x_softc *sc)
sc->phyport[phy] = port;
sc->portphy[port] = phy;
sc->ifp[port] = if_alloc(IFT_ETHER);
+   if (sc->ifp[port] == NULL) {
+   device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
+   err = ENOMEM;
+   break;
+   }
+
sc->ifp[port]->if_softc = sc;
sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
IFF_DRV_RUNNING | IFF_SIMPLEX;

Modified: head/sys/dev/etherswitch/micrel/ksz8995ma.c
==
--- head/sys/dev/etherswitch/micrel/ksz8995ma.c Wed Jan 24 21:26:01 2018
(r328376)
+++ head/sys/dev/etherswitch/micrel/ksz8995ma.c Wed Jan 24 21:33:18 2018
(r328377)
@@ -221,6 +221,12 @@ ksz8995ma_attach_phys(struct ksz8995ma_softc *sc)
sc->ifpport[phy] = port;
sc->portphy[port] = phy;
sc->ifp[port] = if_alloc(IFT_ETHER);

svn commit: r328378 - head/share/man/man8

[Ed Maste]

Author: emaste
Date: Wed Jan 24 21:39:40 2018
New Revision: 328378
URL: https://svnweb.freebsd.org/changeset/base/328378

Log:
  Add efi.8 as a man page link to uefi.8
  
  FreeBSD and industry has been inconsistent in the use of UEFI and EFI.
  They are essentially just different versions of the same specification
  and are often used interchangeably.  Make it easier for users to find
  information by making efi(8) an alias for uefi(8).
  
  Reported by:  imp, jhb

Modified:
  head/share/man/man8/Makefile

Modified: head/share/man/man8/Makefile
==
--- head/share/man/man8/MakefileWed Jan 24 21:33:18 2018
(r328377)
+++ head/share/man/man8/MakefileWed Jan 24 21:39:40 2018
(r328378)
@@ -29,7 +29,8 @@ MLINKS= \
 .if ${MK_NIS} != "no"
 MAN+=  yp.8
 
-MLINKS+=yp.8 NIS.8 \
+MLINKS+=uefi.8 efi.8 \
+   yp.8 NIS.8 \
yp.8 nis.8 \
yp.8 YP.8
 .endif
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Conrad Meyer]

On Wed, Jan 24, 2018 at 1:13 PM, Warner Losh  wrote:
> mallocarray should be the last line of defense, not the only line of
> defense.

Agreed.

> most of the time, it's more correct to say
>
> if (WOULD_OVERFLOW(a,b))
> return EINVAL;
> ptr = mallocarray(a,b...);

Disagree -- the right check to have outside is much more constrained
than just "will this overflow?"  A 10GB allocation request is still
insane on amd64, just for a different reason than on i386.  I think
BDE said something along the lines of max 32 kB for most allocations,
and I don't really disagree with that.  Picking a specific number for
mallocarray itself (other than overflow) restricts the generality,
though.

> since an error return, assuming it's handled correctly is preferable to a
> panic.

Agreed.

> I thought this was more true for NOWAIT than for WAITOK cases, but I've
> realized it's more true always.

Yeah, I think it's equally true of M_WAITOK and M_NOWAIT.

> And that's why I have such a problem with mallocarray: it's only useful when
> people are lazy and haven't checked,

It's useful as a seatbelt.  Empirically, people are not perfect at
doing the checking.  I can understand that it feels like admitting
laziness to accept that we need a final seatbelt check at all, but I
don't think having the seatbelt hurts.

> and then it creates a DoS path for
> things that don't check.

No, again; it doesn't "create" a DoS.  Any DoS path was already
present as a heap corruption path.  The DoS is strictly an
improvement.

> We'll change it now and think we're safe, when we
> still have issues, just different issues than before.

Don't think that, then?  We have replaced some classes of heap
corruption with a DoS.  That's it; nothing more.  I don't think anyone
promoting mallocarray is overrepresenting what it does or claims to
do.

> It may be a necessary
> change, but it certainly isn't sufficient.

I don't disagree.

Best,
Conrad
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328378 - head/share/man/man8

[Rodney W. Grimes]

[ Charset UTF-8 unsupported, converting... ]
> Author: emaste
> Date: Wed Jan 24 21:39:40 2018
> New Revision: 328378
> URL: https://svnweb.freebsd.org/changeset/base/328378
> 
> Log:
>   Add efi.8 as a man page link to uefi.8
>   
>   FreeBSD and industry has been inconsistent in the use of UEFI and EFI.
>   They are essentially just different versions of the same specification
>   and are often used interchangeably.  Make it easier for users to find
>   information by making efi(8) an alias for uefi(8).
>   
>   Reported by:imp, jhb
> 
> Modified:
>   head/share/man/man8/Makefile
> 
> Modified: head/share/man/man8/Makefile
> ==
> --- head/share/man/man8/Makefile  Wed Jan 24 21:33:18 2018
> (r328377)
> +++ head/share/man/man8/Makefile  Wed Jan 24 21:39:40 2018
> (r328378)
> @@ -29,7 +29,8 @@ MLINKS= \
>  .if ${MK_NIS} != "no"
>  MAN+=yp.8
>  
> -MLINKS+=yp.8 NIS.8 \

Uh oh, you yp and NIS are very different that uefi and efi


> +MLINKS+=uefi.8 efi.8 \
> + yp.8 NIS.8 \
>   yp.8 nis.8 \
>   yp.8 YP.8
>  .endif

-- 
Rod Grimes rgri...@freebsd.org
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev

[Warner Losh]

On Wed, Jan 24, 2018 at 2:40 PM, Conrad Meyer  wrote:

> On Wed, Jan 24, 2018 at 1:13 PM, Warner Losh  wrote:
> > mallocarray should be the last line of defense, not the only line of
> > defense.
>
> Agreed.
>
> > most of the time, it's more correct to say
> >
> > if (WOULD_OVERFLOW(a,b))
> > return EINVAL;
> > ptr = mallocarray(a,b...);
>
> Disagree -- the right check to have outside is much more constrained
> than just "will this overflow?"  A 10GB allocation request is still
> insane on amd64, just for a different reason than on i386.  I think
> BDE said something along the lines of max 32 kB for most allocations,
> and I don't really disagree with that.  Picking a specific number for
> mallocarray itself (other than overflow) restricts the generality,
> though.


Well, you're right. there's more context here. You want all these changes
to be nops because the upper layers have effectively vetted the arguments.
Those that don't need something like the above at the very least.
WOULD_OVERFLOW is the least-restrictive version of "is this sane" we have.
In the absence of other data, though, it's the last line.

> since an error return, assuming it's handled correctly is preferable to a
> > panic.
>
> Agreed.
>
> > I thought this was more true for NOWAIT than for WAITOK cases, but I've
> > realized it's more true always.
>
> Yeah, I think it's equally true of M_WAITOK and M_NOWAIT.


Yea. Upon reflection I've come around to this way of thinking.

> And that's why I have such a problem with mallocarray: it's only useful
> when
> > people are lazy and haven't checked,
>
> It's useful as a seatbelt.  Empirically, people are not perfect at
> doing the checking.  I can understand that it feels like admitting
> laziness to accept that we need a final seatbelt check at all, but I
> don't think having the seatbelt hurts.
>
> > and then it creates a DoS path for
> > things that don't check.
>
> No, again; it doesn't "create" a DoS.  Any DoS path was already
> present as a heap corruption path.  The DoS is strictly an
> improvement.


Fair enough. We've traded one problem for another. Depending on your threat
model one or the other may be preferable. A heap overflow that's not
exploitable that the integer overflow may enable might be preferable to a
crash if the system uptime is important though. Both are problems that need
better filtering.


> > We'll change it now and think we're safe, when we
> > still have issues, just different issues than before.
>
> Don't think that, then?  We have replaced some classes of heap
> corruption with a DoS.  That's it; nothing more.  I don't think anyone
> promoting mallocarray is overrepresenting what it does or claims to
> do.


My worry is that people are going through and adding it in a sweep based on
regexp hits without looking more closely to see if there could possibly be
a bigger problem by doing a more in-depth data flow analysis.

> It may be a necessary
> > change, but it certainly isn't sufficient.
>
> I don't disagree.
>

Yea.

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328380 - in head/sys: arm/conf arm/lpc dev/uart modules/uart

[Emmanuel Vadot]

Author: manu
Date: Wed Jan 24 22:04:16 2018
New Revision: 328380
URL: https://svnweb.freebsd.org/changeset/base/328380

Log:
  arm: lpc: Remove support
  
  Code hasn't been touch this it's original commit in 2012 beside api changes.
  
  Reviewed by:  ian
  Differential Revision:https://reviews.freebsd.org/D13625
  Discussed with:   freebsd-...@freebsd.org (no reply)

Deleted:
  head/sys/arm/conf/EA3250
  head/sys/arm/lpc/files.lpc
  head/sys/arm/lpc/if_lpe.c
  head/sys/arm/lpc/if_lpereg.h
  head/sys/arm/lpc/lpc_dmac.c
  head/sys/arm/lpc/lpc_fb.c
  head/sys/arm/lpc/lpc_gpio.c
  head/sys/arm/lpc/lpc_intc.c
  head/sys/arm/lpc/lpc_machdep.c
  head/sys/arm/lpc/lpc_mmc.c
  head/sys/arm/lpc/lpc_ohci.c
  head/sys/arm/lpc/lpc_pll.c
  head/sys/arm/lpc/lpc_pwr.c
  head/sys/arm/lpc/lpc_rtc.c
  head/sys/arm/lpc/lpc_spi.c
  head/sys/arm/lpc/lpc_timer.c
  head/sys/arm/lpc/lpcreg.h
  head/sys/arm/lpc/lpcvar.h
  head/sys/arm/lpc/ssd1289.c
  head/sys/arm/lpc/std.lpc
  head/sys/dev/uart/uart_dev_lpc.c
Modified:
  head/sys/modules/uart/Makefile

Modified: head/sys/modules/uart/Makefile
==
--- head/sys/modules/uart/Makefile  Wed Jan 24 21:48:39 2018
(r328379)
+++ head/sys/modules/uart/Makefile  Wed Jan 24 22:04:16 2018
(r328380)
@@ -6,10 +6,6 @@
 uart_bus_ebus= uart_bus_ebus.c
 .endif
 
-.if ${MACHINE_CPUARCH} == "arm"
-uart_dev_lpc=  uart_dev_lpc.c
-.endif
-
 .if ${MACHINE_CPUARCH} == "aarch64" || ${MACHINE_CPUARCH} == "arm" || \
 ${MACHINE_CPUARCH} == "powerpc" || ${MACHINE_CPUARCH} == "riscv" || \
 ${MACHINE_CPUARCH} == "sparc64"
@@ -33,7 +29,7 @@ KMOD= uart
 SRCS=  uart_bus_acpi.c ${uart_bus_ebus} uart_bus_isa.c uart_bus_pccard.c \
uart_bus_pci.c uart_bus_puc.c uart_bus_scc.c \
uart_core.c ${uart_cpu_machine} uart_dbg.c \
-   ${uart_dev_lpc} ${uart_dev_mvebu} uart_dev_ns8250.c uart_dev_quicc.c \
+   ${uart_dev_mvebu} uart_dev_ns8250.c uart_dev_quicc.c \
uart_dev_sab82532.c uart_dev_z8530.c \
uart_if.c uart_if.h uart_subr.c uart_tty.c
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328381 - in head: contrib/compiler-rt/lib/builtins contrib/llvm/include/llvm/Analysis contrib/llvm/include/llvm/CodeGen contrib/llvm/include/llvm/MC contrib/llvm/include/llvm/Support c...

[Dimitry Andric]

Author: dim
Date: Wed Jan 24 22:35:00 2018
New Revision: 328381
URL: https://svnweb.freebsd.org/changeset/base/328381

Log:
  Upgrade our copies of clang, llvm, lld, lldb, compiler-rt and libc++ to
  6.0.0 (branches/release_60 r323338).
  
  MFC after:3 months
  X-MFC-With:   r327952
  PR:   224669

Modified:
  head/contrib/compiler-rt/lib/builtins/clear_cache.c
  head/contrib/llvm/include/llvm/Analysis/RegionInfoImpl.h
  head/contrib/llvm/include/llvm/CodeGen/SelectionDAGAddressAnalysis.h
  head/contrib/llvm/include/llvm/MC/MCCodeView.h
  head/contrib/llvm/include/llvm/Support/GenericDomTreeConstruction.h
  head/contrib/llvm/lib/CodeGen/GlobalMerge.cpp
  head/contrib/llvm/lib/CodeGen/PeepholeOptimizer.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  head/contrib/llvm/lib/CodeGen/SelectionDAG/SelectionDAGAddressAnalysis.cpp
  head/contrib/llvm/lib/CodeGen/TargetLoweringBase.cpp
  head/contrib/llvm/lib/Linker/IRMover.cpp
  head/contrib/llvm/lib/MC/MCCodeView.cpp
  head/contrib/llvm/lib/Target/AArch64/AArch64InstructionSelector.cpp
  head/contrib/llvm/lib/Target/Hexagon/HexagonISelDAGToDAGHVX.cpp
  head/contrib/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  head/contrib/llvm/lib/Target/PowerPC/PPCISelLowering.h
  head/contrib/llvm/lib/Target/PowerPC/PPCInstrInfo.td
  head/contrib/llvm/lib/Target/X86/X86ISelLowering.cpp
  head/contrib/llvm/lib/Transforms/Scalar/GVNHoist.cpp
  head/contrib/llvm/lib/Transforms/Scalar/StructurizeCFG.cpp
  head/contrib/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  head/contrib/llvm/tools/clang/include/clang/Basic/Attr.td
  head/contrib/llvm/tools/clang/include/clang/Basic/BuiltinsX86.def
  head/contrib/llvm/tools/clang/include/clang/Basic/DiagnosticGroups.td
  head/contrib/llvm/tools/clang/include/clang/Basic/TokenKinds.def
  
head/contrib/llvm/tools/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
  head/contrib/llvm/tools/clang/lib/AST/DeclBase.cpp
  head/contrib/llvm/tools/clang/lib/AST/ODRHash.cpp
  head/contrib/llvm/tools/clang/lib/CodeGen/CGBuiltin.cpp
  head/contrib/llvm/tools/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  head/contrib/llvm/tools/clang/lib/Frontend/InitPreprocessor.cpp
  head/contrib/llvm/tools/clang/lib/Lex/Lexer.cpp
  head/contrib/llvm/tools/clang/lib/Lex/PPCaching.cpp
  head/contrib/llvm/tools/clang/lib/Lex/PPLexerChange.cpp
  head/contrib/llvm/tools/clang/lib/Sema/Scope.cpp
  head/contrib/llvm/tools/clang/lib/Sema/SemaTemplateDeduction.cpp
  head/contrib/llvm/tools/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  head/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  head/contrib/llvm/tools/clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  head/contrib/llvm/tools/lld/COFF/Driver.cpp
  head/contrib/llvm/tools/lld/ELF/SyntheticSections.cpp
  head/contrib/llvm/tools/llvm-readobj/MachODumper.cpp
  head/lib/clang/include/clang/Basic/Version.inc
  head/lib/clang/include/lld/Common/Version.inc
  head/lib/clang/include/llvm/Support/VCSRevision.h
Directory Properties:
  head/contrib/compiler-rt/   (props changed)
  head/contrib/libc++/   (props changed)
  head/contrib/llvm/   (props changed)
  head/contrib/llvm/tools/clang/   (props changed)
  head/contrib/llvm/tools/lld/   (props changed)
  head/contrib/llvm/tools/lldb/   (props changed)

Modified: head/contrib/compiler-rt/lib/builtins/clear_cache.c
==
--- head/contrib/compiler-rt/lib/builtins/clear_cache.c Wed Jan 24 22:04:16 
2018(r328380)
+++ head/contrib/compiler-rt/lib/builtins/clear_cache.c Wed Jan 24 22:35:00 
2018(r328381)
@@ -33,6 +33,11 @@ uintptr_t GetCurrentProcess(void);
   #include 
 #endif
 
+#if defined(__OpenBSD__) && defined(__mips__)
+  #include 
+  #include 
+#endif
+
 #if defined(__linux__) && defined(__mips__)
   #include 
   #include 
@@ -142,6 +147,8 @@ void __clear_cache(void *start, void *end) {
 #else
 syscall(__NR_cacheflush, start, (end_int - start_int), BCACHE);
 #endif
+#elif defined(__mips__) && defined(__OpenBSD__)
+  cacheflush(start, (uintptr_t)end - (uintptr_t)start, BCACHE);
 #elif defined(__aarch64__) && !defined(__APPLE__)
   uint64_t xstart = (uint64_t)(uintptr_t) start;
   uint64_t xend = (uint64_t)(uintptr_t) end;
@@ -156,12 +163,14 @@ void __clear_cache(void *start, void *end) {
* uintptr_t in case this runs in an IPL32 environment.
*/
   const size_t dcache_line_size = 4 << ((ctr_el0 >> 16) & 15);
-  for (addr = xstart; addr < xend; addr += dcache_line_size)
+  for (addr = xstart & ~(dcache_line_size - 1); addr < xend;
+   addr += dcache_line_size)
 __asm __volatile("dc cvau, %0" :: "r"(addr));
   __asm __volatile("dsb ish");
 
   const size_t icache_line_size = 4 << ((ctr_el0 >> 0) & 15);
-  for (addr = xstart; addr < xend; addr += icache_lin

svn commit: r328382 - head/lib/libc/sys

[Kirk McKusick]

Author: mckusick
Date: Wed Jan 24 22:36:21 2018
New Revision: 328382
URL: https://svnweb.freebsd.org/changeset/base/328382

Log:
  Update .Dd missed in -r328304.
  
  Reported by: Bjoern Zeeb (bz)
  MFC with:328304

Modified:
  head/lib/libc/sys/setgroups.2

Modified: head/lib/libc/sys/setgroups.2
==
--- head/lib/libc/sys/setgroups.2   Wed Jan 24 22:35:00 2018
(r328381)
+++ head/lib/libc/sys/setgroups.2   Wed Jan 24 22:36:21 2018
(r328382)
@@ -28,7 +28,7 @@
 .\" @(#)setgroups.28.2 (Berkeley) 4/16/94
 .\" $FreeBSD$
 .\"
-.Dd April 16, 1994
+.Dd January 19, 2018
 .Dt SETGROUPS 2
 .Os
 .Sh NAME
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328383 - head/sbin/fsck_ffs

[Kirk McKusick]

Author: mckusick
Date: Wed Jan 24 23:57:40 2018
New Revision: 328383
URL: https://svnweb.freebsd.org/changeset/base/328383

Log:
  More throughly integrate libufs into fsck_ffs by using its cgput()
  routine to write out the cylinder groups rather than recreating the
  calculation of the cylinder-group check hash in fsck_ffs.
  
  No functional change intended.

Modified:
  head/sbin/fsck_ffs/fsck.h
  head/sbin/fsck_ffs/fsutil.c
  head/sbin/fsck_ffs/gjournal.c
  head/sbin/fsck_ffs/setup.c
  head/sbin/fsck_ffs/suj.c

Modified: head/sbin/fsck_ffs/fsck.h
==
--- head/sbin/fsck_ffs/fsck.h   Wed Jan 24 22:36:21 2018(r328382)
+++ head/sbin/fsck_ffs/fsck.h   Wed Jan 24 23:57:40 2018(r328383)
@@ -327,6 +327,7 @@ extern char skipclean;  /* skip clean file 
systems if 
 extern int fsmodified; /* 1 => write done to file system */
 extern int fsreadfd;   /* file descriptor for reading file 
system */
 extern int fswritefd;  /* file descriptor for writing file 
system */
+extern struct  uufsd disk; /* libufs user-ufs disk structure */
 extern int surrender;  /* Give up if reads fail */
 extern int wantrestart;/* Restart fsck on early termination */
 

Modified: head/sbin/fsck_ffs/fsutil.c
==
--- head/sbin/fsck_ffs/fsutil.c Wed Jan 24 22:36:21 2018(r328382)
+++ head/sbin/fsck_ffs/fsutil.c Wed Jan 24 23:57:40 2018(r328383)
@@ -352,20 +352,6 @@ flush(int fd, struct bufarea *bp)
 
if (!bp->b_dirty)
return;
-   /*
-* Calculate any needed check hashes.
-*/
-   switch (bp->b_type) {
-   case BT_CYLGRP:
-   if ((sblock.fs_metackhash & CK_CYLGRP) == 0)
-   break;
-   bp->b_un.b_cg->cg_ckhash = 0;
-   bp->b_un.b_cg->cg_ckhash =
-   calculate_crc32c(~0L, bp->b_un.b_buf, bp->b_size);
-   break;
-   default:
-   break;
-   }
bp->b_dirty = 0;
if (fswritefd < 0) {
pfatal("WRITING IN READ_ONLY MODE.\n");
@@ -376,13 +362,30 @@ flush(int fd, struct bufarea *bp)
(bp->b_errs == bp->b_size / dev_bsize) ? "" : "PARTIALLY ",
(long long)bp->b_bno);
bp->b_errs = 0;
-   blwrite(fd, bp->b_un.b_buf, bp->b_bno, bp->b_size);
-   if (bp != &sblk)
-   return;
-   for (i = 0, j = 0; i < sblock.fs_cssize; i += sblock.fs_bsize, j++) {
-   blwrite(fswritefd, (char *)sblock.fs_csp + i,
-   fsbtodb(&sblock, sblock.fs_csaddr + j * sblock.fs_frag),
-   MIN(sblock.fs_cssize - i, sblock.fs_bsize));
+   /*
+* Write using the appropriate function.
+*/
+   switch (bp->b_type) {
+   case BT_SUPERBLK:
+   if (bp != &sblk)
+   pfatal("BUFFER 0x%x DOES NOT MATCH SBLK 0x%x\n",
+   (u_int)bp, (u_int)&sblk);
+   blwrite(fd, bp->b_un.b_buf, bp->b_bno, bp->b_size);
+   for (i = 0, j = 0; i < sblock.fs_cssize; i += sblock.fs_bsize,
+  j++) {
+   blwrite(fswritefd, (char *)sblock.fs_csp + i,
+   fsbtodb(&sblock,
+   sblock.fs_csaddr + j * sblock.fs_frag),
+   MIN(sblock.fs_cssize - i, sblock.fs_bsize));
+   }
+   break;
+   case BT_CYLGRP:
+   if (cgput(&disk, (struct cg *)bp->b_un.b_buf) == 0)
+   fsmodified = 1;
+   break;
+   default:
+   blwrite(fd, bp->b_un.b_buf, bp->b_bno, bp->b_size);
+   break;
}
 }
 

Modified: head/sbin/fsck_ffs/gjournal.c
==
--- head/sbin/fsck_ffs/gjournal.c   Wed Jan 24 22:36:21 2018
(r328382)
+++ head/sbin/fsck_ffs/gjournal.c   Wed Jan 24 23:57:40 2018
(r328383)
@@ -91,7 +91,7 @@ static unsigned ncgs = 0;
 static LIST_HEAD(, cgchain) cglist = LIST_HEAD_INITIALIZER(cglist);
 
 static const char *devnam;
-static struct uufsd *disk = NULL;
+static struct uufsd *diskp = NULL;
 static struct fs *fs = NULL;
 struct ufs2_dinode ufs2_zino;
 
@@ -107,7 +107,7 @@ getcg(int cg)
 {
struct cgchain *cgc;
 
-   assert(disk != NULL && fs != NULL);
+   assert(diskp != NULL && fs != NULL);
LIST_FOREACH(cgc, &cglist, cgc_next) {
if (cgc->cgc_cg.cg_cgx == cg) {
//printf("%s: Found cg=%d\n", __func__, cg);
@@ -134,7 +134,7 @@ getcg(int cg)
if (cgc == NULL)
err(1, "malloc(%zu)", sizeof(*cgc));
}
-   if (cgget(disk, cg, &cgc->cgc_cg) == -1)
+   if (cgge

Re: svn commit: r328313 - head/sys/netpfil/pf

[Gleb Smirnoff]

  Hi Kristof,

On Wed, Jan 24, 2018 at 04:29:17AM +, Kristof Provost wrote:
K> Author: kp
K> Date: Wed Jan 24 04:29:16 2018
K> New Revision: 328313
K> URL: https://svnweb.freebsd.org/changeset/base/328313
K> 
K> Log:
K>   pf: States have at least two references
K>   
K>   pf_unlink_state() releases a reference to the state without checking if
K>   this is the last reference. It can't be, because pf_state_insert()
K>   initialises it to two. KASSERT() that this is always the case.
K>   
K>   CID:   1347140
K> 
K> Modified:
K>   head/sys/netpfil/pf/pf.c
K> 
K> Modified: head/sys/netpfil/pf/pf.c
K> 
==
K> --- head/sys/netpfil/pf/pf.c Wed Jan 24 03:09:56 2018(r328312)
K> +++ head/sys/netpfil/pf/pf.c Wed Jan 24 04:29:16 2018(r328313)
K> @@ -1613,6 +1613,7 @@ int
K>  pf_unlink_state(struct pf_state *s, u_int flags)
K>  {
K>  struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
K> +int last;
K>  
K>  if ((flags & PF_ENTER_LOCKED) == 0)
K>  PF_HASHROW_LOCK(ih);
K> @@ -1653,7 +1654,8 @@ pf_unlink_state(struct pf_state *s, u_int flags)
K>  PF_HASHROW_UNLOCK(ih);
K>  
K>  pf_detach_state(s);
K> -refcount_release(&s->refs);
K> +last = refcount_release(&s->refs);
K> +KASSERT(last == 0, ("Incorrect state reference count"));
K>  
K>  return (pf_release_state(s));
K>  }

IMHO, we shouldn't emit extra code to please Coverity. We can mark it
as a false positive in the interface. It may make sense to add a comment
for a human to explain why return isn't checked here.

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328313 - head/sys/netpfil/pf

[Kristof Provost]

On 25 Jan 2018, at 11:13, Gleb Smirnoff wrote:

On Wed, Jan 24, 2018 at 04:29:17AM +, Kristof Provost wrote:
K> Author: kp
K> Date: Wed Jan 24 04:29:16 2018
K> New Revision: 328313
K> URL: https://svnweb.freebsd.org/changeset/base/328313
K>
K> Log:
K>   pf: States have at least two references
K>
K>   pf_unlink_state() releases a reference to the state without 
checking if
K>   this is the last reference. It can't be, because 
pf_state_insert()

K>   initialises it to two. KASSERT() that this is always the case.
K>
K>   CID:1347140
K>



K> + last = refcount_release(&s->refs);
K> + KASSERT(last == 0, ("Incorrect state reference count"));
K>
K>   return (pf_release_state(s));
K>  }

IMHO, we shouldn't emit extra code to please Coverity. We can mark it
as a false positive in the interface. It may make sense to add a 
comment

for a human to explain why return isn't checked here.

I find the KASSERT() to be a good way of verifying that this assumption 
is in fact correct.

You do have a good point about the comment.

Regards,
Kristof
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328377 - in head/sys/dev/etherswitch: arswitch e6000sw infineon ip17x micrel mtkswitch rtl8366 ukswitch

[Gleb Smirnoff]

  Hi Michael,

I already replied to Dmitry that this patch isn't correct.
The if_alloc() shall never fail. All checks like that needs
to be removed. Inside the if_alloc() the check for L2COM
allocation should also be removed. Everything happens with
M_WAITOK in this path.

On Wed, Jan 24, 2018 at 09:33:18PM +, Michael Zhilin wrote:
M> Author: mizhka
M> Date: Wed Jan 24 21:33:18 2018
M> New Revision: 328377
M> URL: https://svnweb.freebsd.org/changeset/base/328377
M> 
M> Log:
M>   [etherswitch] check if_alloc returns NULL
M>   
M>   This patch is cosmetic. It checks if allocation of ifnet structure failed.
M>   It's better to have this check rather than assume positive scenario.
M>   
M>   Submitted by:  Dmitry Luhtionov 
M>   Reported by:   Dmitry Luhtionov 
M> 
M> Modified:
M>   head/sys/dev/etherswitch/arswitch/arswitch.c
M>   head/sys/dev/etherswitch/e6000sw/e6060sw.c
M>   head/sys/dev/etherswitch/infineon/adm6996fc.c
M>   head/sys/dev/etherswitch/ip17x/ip17x.c
M>   head/sys/dev/etherswitch/micrel/ksz8995ma.c
M>   head/sys/dev/etherswitch/mtkswitch/mtkswitch.c
M>   head/sys/dev/etherswitch/rtl8366/rtl8366rb.c
M>   head/sys/dev/etherswitch/ukswitch/ukswitch.c
M> 
M> Modified: head/sys/dev/etherswitch/arswitch/arswitch.c
M> 
==
M> --- head/sys/dev/etherswitch/arswitch/arswitch.c Wed Jan 24 21:26:01 
2018(r328376)
M> +++ head/sys/dev/etherswitch/arswitch/arswitch.c Wed Jan 24 21:33:18 
2018(r328377)
M> @@ -177,6 +177,12 @@ arswitch_attach_phys(struct arswitch_softc *sc)
M>  snprintf(name, IFNAMSIZ, "%sport", device_get_nameunit(sc->sc_dev));
M>  for (phy = 0; phy < sc->numphys; phy++) {
M>  sc->ifp[phy] = if_alloc(IFT_ETHER);
M> +if (sc->ifp[phy] == NULL) {
M> +device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
M> +err = ENOMEM;
M> +break;
M> +}
M> +
M>  sc->ifp[phy]->if_softc = sc;
M>  sc->ifp[phy]->if_flags |= IFF_UP | IFF_BROADCAST |
M>  IFF_DRV_RUNNING | IFF_SIMPLEX;
M> 
M> Modified: head/sys/dev/etherswitch/e6000sw/e6060sw.c
M> 
==
M> --- head/sys/dev/etherswitch/e6000sw/e6060sw.c   Wed Jan 24 21:26:01 
2018(r328376)
M> +++ head/sys/dev/etherswitch/e6000sw/e6060sw.c   Wed Jan 24 21:33:18 
2018(r328377)
M> @@ -218,6 +218,12 @@ e6060sw_attach_phys(struct e6060sw_softc *sc)
M>  sc->ifpport[phy] = port;
M>  sc->portphy[port] = phy;
M>  sc->ifp[port] = if_alloc(IFT_ETHER);
M> +if (sc->ifp[port] == NULL) {
M> +device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
M> +err = ENOMEM;
M> +break;
M> +}
M> +
M>  sc->ifp[port]->if_softc = sc;
M>  sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
M>  IFF_DRV_RUNNING | IFF_SIMPLEX;
M> 
M> Modified: head/sys/dev/etherswitch/infineon/adm6996fc.c
M> 
==
M> --- head/sys/dev/etherswitch/infineon/adm6996fc.cWed Jan 24 21:26:01 
2018(r328376)
M> +++ head/sys/dev/etherswitch/infineon/adm6996fc.cWed Jan 24 21:33:18 
2018(r328377)
M> @@ -175,6 +175,12 @@ adm6996fc_attach_phys(struct adm6996fc_softc *sc)
M>  sc->ifpport[phy] = port;
M>  sc->portphy[port] = phy;
M>  sc->ifp[port] = if_alloc(IFT_ETHER);
M> +if (sc->ifp[port] == NULL) {
M> +device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
M> +err = ENOMEM;
M> +break;
M> +}
M> +
M>  sc->ifp[port]->if_softc = sc;
M>  sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
M>  IFF_DRV_RUNNING | IFF_SIMPLEX;
M> 
M> Modified: head/sys/dev/etherswitch/ip17x/ip17x.c
M> 
==
M> --- head/sys/dev/etherswitch/ip17x/ip17x.c   Wed Jan 24 21:26:01 2018
(r328376)
M> +++ head/sys/dev/etherswitch/ip17x/ip17x.c   Wed Jan 24 21:33:18 2018
(r328377)
M> @@ -174,6 +174,12 @@ ip17x_attach_phys(struct ip17x_softc *sc)
M>  sc->phyport[phy] = port;
M>  sc->portphy[port] = phy;
M>  sc->ifp[port] = if_alloc(IFT_ETHER);
M> +if (sc->ifp[port] == NULL) {
M> +device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
M> +err = ENOMEM;
M> +break;
M> +}
M> +
M>  sc->ifp[port]->if_softc = sc;
M>  sc->ifp[port]->if_flags |= IFF_UP | IFF_BROADCAST |
M>  IFF_DRV_RUNNING | IFF_SIMPLEX;
M> 
M> Modified: head/sy

Re: svn commit: r328313 - head/sys/netpfil/pf

[Gleb Smirnoff]

On Thu, Jan 25, 2018 at 11:16:23AM +1100, Kristof Provost wrote:
K> On 25 Jan 2018, at 11:13, Gleb Smirnoff wrote:
K> > On Wed, Jan 24, 2018 at 04:29:17AM +, Kristof Provost wrote:
K> > K> Author: kp
K> > K> Date: Wed Jan 24 04:29:16 2018
K> > K> New Revision: 328313
K> > K> URL: https://svnweb.freebsd.org/changeset/base/328313
K> > K>
K> > K> Log:
K> > K>   pf: States have at least two references
K> > K>
K> > K>   pf_unlink_state() releases a reference to the state without 
K> > checking if
K> > K>   this is the last reference. It can't be, because 
K> > pf_state_insert()
K> > K>   initialises it to two. KASSERT() that this is always the case.
K> > K>
K> > K>   CID:  1347140
K> > K>
K> 
K> > K> +   last = refcount_release(&s->refs);
K> > K> +   KASSERT(last == 0, ("Incorrect state reference count"));
K> > K>
K> > K> return (pf_release_state(s));
K> > K>  }
K> >
K> > IMHO, we shouldn't emit extra code to please Coverity. We can mark it
K> > as a false positive in the interface. It may make sense to add a 
K> > comment
K> > for a human to explain why return isn't checked here.
K> >
K> I find the KASSERT() to be a good way of verifying that this assumption 
K> is in fact correct.
K> You do have a good point about the comment.

Now Coverity will report you about a write-only variable, if Coverity has
a run with INVARIANTS undefined. There should be a better way for doing
that. I don't see it right now though :)

-- 
Gleb Smirnoff
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328313 - head/sys/netpfil/pf

[Ian Lepore]

On Wed, 2018-01-24 at 16:13 -0800, Gleb Smirnoff wrote:
>   Hi Kristof,
> 
> On Wed, Jan 24, 2018 at 04:29:17AM +, Kristof Provost wrote:
> K> Author: kp
> K> Date: Wed Jan 24 04:29:16 2018
> K> New Revision: 328313
> K> URL: https://svnweb.freebsd.org/changeset/base/328313
> K> 
> K> Log:
> K>   pf: States have at least two references
> K>   
> K>   pf_unlink_state() releases a reference to the state without
> checking if
> K>   this is the last reference. It can't be, because
> pf_state_insert()
> K>   initialises it to two. KASSERT() that this is always the case.
> K>   
> K>   CID: 1347140
> K> 
> K> Modified:
> K>   head/sys/netpfil/pf/pf.c
> K> 
> K> Modified: head/sys/netpfil/pf/pf.c
> K>
> =
> =
> K> --- head/sys/netpfil/pf/pf.c   Wed Jan 24 03:09:56 2018
> (r328312)
> K> +++ head/sys/netpfil/pf/pf.c   Wed Jan 24 04:29:16 2018
> (r328313)
> K> @@ -1613,6 +1613,7 @@ int
> K>  pf_unlink_state(struct pf_state *s, u_int flags)
> K>  {
> K>    struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
> K> +  int last;
> K>  
> K>    if ((flags & PF_ENTER_LOCKED) == 0)
> K>    PF_HASHROW_LOCK(ih);
> K> @@ -1653,7 +1654,8 @@ pf_unlink_state(struct pf_state *s, u_int
> flags)
> K>    PF_HASHROW_UNLOCK(ih);
> K>  
> K>    pf_detach_state(s);
> K> -  refcount_release(&s->refs);
> K> +  last = refcount_release(&s->refs);
> K> +  KASSERT(last == 0, ("Incorrect state reference count"));
> K>  
> K>    return (pf_release_state(s));
> K>  }
> 
> IMHO, we shouldn't emit extra code to please Coverity. We can mark it
> as a false positive in the interface. It may make sense to add a
> comment
> for a human to explain why return isn't checked here.
> 

Not to mention that when KASSERT compiles to nothing, what you're left
with is a "defined but not used" warning for 'last'.

-- Ian
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328313 - head/sys/netpfil/pf

[Kristof Provost]

On 25 Jan 2018, at 11:34, Ian Lepore wrote:

On Wed, 2018-01-24 at 16:13 -0800, Gleb Smirnoff wrote:

(r328313)
K> @@ -1613,6 +1613,7 @@ int
K>  pf_unlink_state(struct pf_state *s, u_int flags)
K>  {
K>   struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
K> + int last;
K>  
K>   if ((flags & PF_ENTER_LOCKED) == 0)
K>   PF_HASHROW_LOCK(ih);
K> @@ -1653,7 +1654,8 @@ pf_unlink_state(struct pf_state *s, u_int
flags)
K>   PF_HASHROW_UNLOCK(ih);
K>  
K>   pf_detach_state(s);
K> - refcount_release(&s->refs);
K> + last = refcount_release(&s->refs);
K> + KASSERT(last == 0, ("Incorrect state reference count"));
K>  
K>   return (pf_release_state(s));
K>  }

IMHO, we shouldn't emit extra code to please Coverity. We can mark it
as a false positive in the interface. It may make sense to add a
comment
for a human to explain why return isn't checked here.



Not to mention that when KASSERT compiles to nothing, what you're left
with is a "defined but not used" warning for 'last'.

I’d really like to keep the KASSERT(), because this is the sort of 
thing that could go wrong, and the assertion would be helpful.


I suppose I could wrap last in #ifdef INVARIANTS, but that’s rather 
ugly too.


Asserting that the refcount is at least 1 when entering 
pf_release_state() would express the same, but that’s also 
problematic.
Of course, errors should trigger the KASSERT() in refcount_release(), so 
I think I may have convinced myself that the KASSERT() can in fact be 
removed and replaced with (void)refcount_release() and a comment 
explaining why this refcount_release() can never return 1.


Regards,
Kristof
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328313 - head/sys/netpfil/pf

[Warner Losh]

On Jan 24, 2018 6:08 PM, "Kristof Provost"  wrote:

On 25 Jan 2018, at 11:34, Ian Lepore wrote:

> On Wed, 2018-01-24 at 16:13 -0800, Gleb Smirnoff wrote:
>
>> (r328313)
>> K> @@ -1613,6 +1613,7 @@ int
>> K>  pf_unlink_state(struct pf_state *s, u_int flags)
>> K>  {
>> K>  struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
>> K> +int last;
>> K>
>> K>  if ((flags & PF_ENTER_LOCKED) == 0)
>> K>  PF_HASHROW_LOCK(ih);
>> K> @@ -1653,7 +1654,8 @@ pf_unlink_state(struct pf_state *s, u_int
>> flags)
>> K>  PF_HASHROW_UNLOCK(ih);
>> K>
>> K>  pf_detach_state(s);
>> K> -refcount_release(&s->refs);
>> K> +last = refcount_release(&s->refs);
>> K> +KASSERT(last == 0, ("Incorrect state reference count"));
>> K>
>> K>  return (pf_release_state(s));
>> K>  }
>>
>> IMHO, we shouldn't emit extra code to please Coverity. We can mark it
>> as a false positive in the interface. It may make sense to add a
>> comment
>> for a human to explain why return isn't checked here.
>>
>>
> Not to mention that when KASSERT compiles to nothing, what you're left
> with is a "defined but not used" warning for 'last'.
>
> I’d really like to keep the KASSERT(), because this is the sort of thing
that could go wrong, and the assertion would be helpful.

I suppose I could wrap last in #ifdef INVARIANTS, but that’s rather ugly
too.

Asserting that the refcount is at least 1 when entering pf_release_state()
would express the same, but that’s also problematic.
Of course, errors should trigger the KASSERT() in refcount_release(), so I
think I may have convinced myself that the KASSERT() can in fact be removed
and replaced with (void)refcount_release() and a comment explaining why
this refcount_release() can never return 1.


It would be good to have a Coverty cheer sheet so we know how to annotate
this stuff correctly in the source. I've started fumbling my way through
for the boot loader and devmatch changes...

Warner
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328387 - head/sys/cam/scsi

[Warner Losh]

Author: imp
Date: Thu Jan 25 02:52:44 2018
New Revision: 328387
URL: https://svnweb.freebsd.org/changeset/base/328387

Log:
  Minor whitespace cleanup to remove leading space before tab. No
  functional changes.

Modified:
  head/sys/cam/scsi/scsi_da.c

Modified: head/sys/cam/scsi/scsi_da.c
==
--- head/sys/cam/scsi/scsi_da.c Thu Jan 25 02:45:21 2018(r328386)
+++ head/sys/cam/scsi/scsi_da.c Thu Jan 25 02:52:44 2018(r328387)
@@ -152,7 +152,7 @@ typedef enum {
DA_CCB_BUFFER_IO= 0x07,
DA_CCB_DUMP = 0x0A,
DA_CCB_DELETE   = 0x0B,
-   DA_CCB_TUR  = 0x0C,
+   DA_CCB_TUR  = 0x0C,
DA_CCB_PROBE_ZONE   = 0x0D,
DA_CCB_PROBE_ATA_LOGDIR = 0x0E,
DA_CCB_PROBE_ATA_IDDIR  = 0x0F,
@@ -303,7 +303,7 @@ struct da_softc {
int  error_inject;
int  trim_max_ranges;
int  delete_available;  /* Delete methods possibly available */
-   da_zone_modezone_mode;
+   da_zone_modezone_mode;
da_zone_interface   zone_interface;
da_zone_flags   zone_flags;
struct ata_gp_log_dir   ata_logdir;
@@ -313,7 +313,7 @@ struct da_softc {
uint64_toptimal_seq_zones;
uint64_toptimal_nonseq_zones;
uint64_tmax_seq_zones;
-   u_int   maxio;
+   u_int   maxio;
uint32_tunmap_max_ranges;
uint32_tunmap_max_lba; /* Max LBAs in UNMAP req */
uint32_tunmap_gran;
@@ -502,16 +502,16 @@ static struct da_quirk_entry da_quirk_table[] =
{T_DIRECT, SIP_MEDIA_REMOVABLE, "Generic*", "USB Flash Disk*",
"*"}, /*quirks*/ DA_Q_NO_SYNC_CACHE
},
-   {
-   /*
-* Creative Nomad MUVO mp3 player (USB)
-* PR: kern/53094
-*/
-   {T_DIRECT, SIP_MEDIA_REMOVABLE, "CREATIVE", "NOMAD_MUVO", "*"},
-   /*quirks*/ DA_Q_NO_SYNC_CACHE|DA_Q_NO_PREVENT
-   },
{
/*
+* Creative Nomad MUVO mp3 player (USB)
+* PR: kern/53094
+*/
+   {T_DIRECT, SIP_MEDIA_REMOVABLE, "CREATIVE", "NOMAD_MUVO", "*"},
+   /*quirks*/ DA_Q_NO_SYNC_CACHE|DA_Q_NO_PREVENT
+   },
+   {
+   /*
 * Jungsoft NEXDISK USB flash key
 * PR: kern/54737
 */
@@ -557,7 +557,7 @@ static struct da_quirk_entry da_quirk_table[] =
 */
{T_DIRECT, SIP_MEDIA_REMOVABLE, "iRiver", "iFP*", "*"},
/*quirks*/ DA_Q_NO_SYNC_CACHE
-   },
+   },
{
/*
 * Frontier Labs NEX IA+ Digital Audio Player, rev 1.10/0.01
@@ -3947,7 +3947,7 @@ cmd6workaround(union ccb *ccb)
 
xpt_print(ccb->ccb_h.path, "READ(6)/WRITE(6) not supported, "
"increasing minimum_cmd_size to 10.\n");
-   softc->minimum_cmd_size = 10;
+   softc->minimum_cmd_size = 10;
 
bcopy(cdb, &cmd6, sizeof(struct scsi_rw_6));
cmd10 = (struct scsi_rw_10 *)cdb;
@@ -3961,7 +3961,7 @@ cmd6workaround(union ccb *ccb)
 
/* Requeue request, unfreezing queue if necessary */
frozen = (ccb->ccb_h.status & CAM_DEV_QFRZN) != 0;
-   ccb->ccb_h.status = CAM_REQUEUE_REQ;
+   ccb->ccb_h.status = CAM_REQUEUE_REQ;
xpt_action(ccb);
if (frozen) {
cam_release_devq(ccb->ccb_h.path,
@@ -5436,17 +5436,17 @@ daerror(union ccb *ccb, u_int32_t cam_flags, u_int32_t
periph = xpt_path_periph(ccb->ccb_h.path);
softc = (struct da_softc *)periph->softc;
 
-   /*
+   /*
 * Automatically detect devices that do not support
-* READ(6)/WRITE(6) and upgrade to using 10 byte cdbs.
-*/
+* READ(6)/WRITE(6) and upgrade to using 10 byte cdbs.
+*/
error = 0;
if ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_INVALID) {
error = cmd6workaround(ccb);
} else if (scsi_extract_sense_ccb(ccb,
&error_code, &sense_key, &asc, &ascq)) {
if (sense_key == SSD_KEY_ILLEGAL_REQUEST)
-   error = cmd6workaround(ccb);
+   error = cmd6workaround(ccb);
/*
 * If the target replied with CAPACITY DATA HAS CHANGED UA,
 * query the capacity and notify upper layers.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r328313 - head/sys/netpfil/pf

[Kristof Provost]

On 25 Jan 2018, at 12:08, Kristof Provost wrote:

On 25 Jan 2018, at 11:34, Ian Lepore wrote:

On Wed, 2018-01-24 at 16:13 -0800, Gleb Smirnoff wrote:

(r328313)
K> @@ -1613,6 +1613,7 @@ int
K>  pf_unlink_state(struct pf_state *s, u_int flags)
K>  {
K>   struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
K> + int last;
K>  
K>   if ((flags & PF_ENTER_LOCKED) == 0)
K>   PF_HASHROW_LOCK(ih);
K> @@ -1653,7 +1654,8 @@ pf_unlink_state(struct pf_state *s, u_int
flags)
K>   PF_HASHROW_UNLOCK(ih);
K>  
K>   pf_detach_state(s);
K> - refcount_release(&s->refs);
K> + last = refcount_release(&s->refs);
K> + KASSERT(last == 0, ("Incorrect state reference count"));
K>  
K>   return (pf_release_state(s));
K>  }

IMHO, we shouldn't emit extra code to please Coverity. We can mark 
it

as a false positive in the interface. It may make sense to add a
comment
for a human to explain why return isn't checked here.



Not to mention that when KASSERT compiles to nothing, what you're 
left

with is a "defined but not used" warning for 'last'.

I’d really like to keep the KASSERT(), because this is the sort of 
thing that could go wrong, and the assertion would be helpful.


I suppose I could wrap last in #ifdef INVARIANTS, but that’s rather 
ugly too.


Asserting that the refcount is at least 1 when entering 
pf_release_state() would express the same, but that’s also 
problematic.
Of course, errors should trigger the KASSERT() in refcount_release(), 
so I think I may have convinced myself that the KASSERT() can in fact 
be removed and replaced with (void)refcount_release() and a comment 
explaining why this refcount_release() can never return 1.



So this:

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 55ae1145835..0dbf1fe7f66 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -1623,7 +1623,6 @@ int
 pf_unlink_state(struct pf_state *s, u_int flags)
 {
struct pf_idhash *ih = &V_pf_idhash[PF_IDHASH(s)];
-   int last;

if ((flags & PF_ENTER_LOCKED) == 0)
PF_HASHROW_LOCK(ih);
@@ -1664,8 +1663,9 @@ pf_unlink_state(struct pf_state *s, u_int flags)
PF_HASHROW_UNLOCK(ih);

pf_detach_state(s);
-   last = refcount_release(&s->refs);
-   KASSERT(last == 0, ("Incorrect state reference count"));
	+   /* pf_state_insert() initialises refs to 2, so we can never 
release the

+* last reference here, only in pf_release_state(). */
+   (void)refcount_release(&s->refs);

return (pf_release_state(s));
 }

I do assume that (void) will tell Coverity I’m deliberately ignoring 
the return value. It’s a fairly common idiom, so I’d expect it to 
understand.


Regards,
Kristof
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328388 - head/share/man/man8

[Li-Wen Hsu]

Author: lwhsu (ports committer)
Date: Thu Jan 25 05:15:44 2018
New Revision: 328388
URL: https://svnweb.freebsd.org/changeset/base/328388

Log:
  Fix manual page install on non-amd64
  
  Reviewed by:  emaste, imp
  Differential Revision:https://reviews.freebsd.org/D14038

Modified:
  head/share/man/man8/Makefile

Modified: head/share/man/man8/Makefile
==
--- head/share/man/man8/MakefileThu Jan 25 02:52:44 2018
(r328387)
+++ head/share/man/man8/MakefileThu Jan 25 05:15:44 2018
(r328388)
@@ -29,14 +29,15 @@ MLINKS= \
 .if ${MK_NIS} != "no"
 MAN+=  yp.8
 
-MLINKS+=uefi.8 efi.8 \
-   yp.8 NIS.8 \
+MLINKS+=yp.8 NIS.8 \
yp.8 nis.8 \
yp.8 YP.8
 .endif
 
 .if ${MACHINE_CPUARCH} == "amd64"
 _uefi.8= uefi.8
+
+MLINKS+=uefi.8 efi.8
 .endif
 
 .include 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328389 - head/sys/dev/etherswitch/rtl8366

[Michael Zhilin]

Author: mizhka
Date: Thu Jan 25 05:48:42 2018
New Revision: 328389
URL: https://svnweb.freebsd.org/changeset/base/328389

Log:
  [etherswitch] fix LINT build for rtl8366rb
  
  Build with rtl8366rb has been broken due to incorrect retrieval of pointer
  to device_t.
  
  Reported by:  lwhsu
  Differential Revision:https://reviews.freebsd.org/D14044

Modified:
  head/sys/dev/etherswitch/rtl8366/rtl8366rb.c

Modified: head/sys/dev/etherswitch/rtl8366/rtl8366rb.c
==
--- head/sys/dev/etherswitch/rtl8366/rtl8366rb.cThu Jan 25 05:15:44 
2018(r328388)
+++ head/sys/dev/etherswitch/rtl8366/rtl8366rb.cThu Jan 25 05:48:42 
2018(r328389)
@@ -240,7 +240,7 @@ rtl8366rb_attach(device_t dev)
for (i = 0; i < sc->numphys; i++) {
sc->ifp[i] = if_alloc(IFT_ETHER);
if (sc->ifp[i] == NULL) {
-   device_printf(sc->sc_dev, "couldn't allocate ifnet 
structure\n");
+   device_printf(dev, "couldn't allocate ifnet 
structure\n");
err = ENOMEM;
break;
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r328390 - head/sys/kern

[Li-Wen Hsu]

Author: lwhsu (ports committer)
Date: Thu Jan 25 06:37:14 2018
New Revision: 328390
URL: https://svnweb.freebsd.org/changeset/base/328390

Log:
  Fix build for architectures where size_t is not unsigned long
  
  Reviewed by:  cem
  Differential Revision:https://reviews.freebsd.org/D14045

Modified:
  head/sys/kern/kern_malloc.c

Modified: head/sys/kern/kern_malloc.c
==
--- head/sys/kern/kern_malloc.c Thu Jan 25 05:48:42 2018(r328389)
+++ head/sys/kern/kern_malloc.c Thu Jan 25 06:37:14 2018(r328390)
@@ -453,7 +453,7 @@ contigfree(void *addr, unsigned long size, struct mall
 
 #ifdef MALLOC_DEBUG
 static int
-malloc_dbg(caddr_t *vap, unsigned long *sizep, struct malloc_type *mtp,
+malloc_dbg(caddr_t *vap, size_t *sizep, struct malloc_type *mtp,
 int flags)
 {
 #ifdef INVARIANTS
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"