const unsigned int nreloc = eb->exec[i].relocation_count;
...
size = nreloc * sizeof(*relocs);
relocs = kvmalloc_array(1, size, GFP_KERNEL);
So something isn't checking the "relocation_count" size that I assume is
coming in from the ioctl?
-Kees
--
Kees Cook
const unsigned int nreloc = eb->exec[i].relocation_count;
...
size = nreloc * sizeof(*relocs);
relocs = kvmalloc_array(1, size, GFP_KERNEL);
So something isn't checking the "relocation_count" size that I assume is
coming in from the ioctl?
-Kees
--
Kees Cook
.org/lkml/ca+55afwqed_d40g4mucssvrzzrfpujt74vc6pppb675hynx...@mail.gmail.com/
--
Kees Cook
.
-Kees
[1]
https://lore.kernel.org/linux-hardening/20240519190422.work.715-k...@kernel.org/
Kees Cook (2):
exec: Add KUnit test for bprm_stack_limits()
exec: Avoid pathological argc, envc, and bprm->p values
MAINTAINERS | 2 +
fs/Kconfig.binfmt | 8 +++
fs/exec.c |
Make sure nothing goes wrong with the string counters or the bprm's
belief about the stack pointer. Add checks and matching self-tests.
For 32-bit validation, this was run under 32-bit UML:
$ tools/testing/kunit/kunit.py run --make_options SUBARCH=i386 exec
Signed-off-by: Kees Cook
---
Cc: Eric
Since bprm_stack_limits() operates with very limited side-effects, add
it as the first exec.c KUnit test. Add to Kconfig and adjust MAINTAINERS
file to include it.
Tested on 64-bit UML:
$ tools/testing/kunit/kunit.py run exec
Signed-off-by: Kees Cook
---
Cc: Eric Biederman
Cc: Justin Stitt
Cc
Convert the runtime tests of hardened usercopy to standard KUnit tests.
Co-developed-by: Vitor Massaru Iha
Signed-off-by: Vitor Massaru Iha
Link: https://lore.kernel.org/r/20200721174654.72132-1-vi...@massaru.org
Signed-off-by: Kees Cook
---
MAINTAINERS| 1
sic infrastructure for adding Mark's much more complete usercopy tests.
-Kees
[1] https://lore.kernel.org/lkml/20230321122514.1743889-2-mark.rutl...@arm.com/
Kees Cook (2):
kunit: test: Add vm_mmap() allocation resource manager
usercopy: Convert test_user_copy to KUnit test
MAINTAIN
://lore.kernel.org/lkml/20230321122514.1743889-2-mark.rutl...@arm.com/ [1]
Co-developed-by: Mark Rutland
Signed-off-by: Mark Rutland
Signed-off-by: Kees Cook
---
include/kunit/test.h | 17 ++
lib/kunit/test.c | 139 ++-
2 files changed, 155 insertions(+), 1
; So, let's use this helper here.
>
> Signed-off-by: Guilherme G. Piccoli
Ah yeah, good idea!
Reviewed-by: Kees Cook
--
Kees Cook
On Sat, May 18, 2024 at 10:23:54PM +0200, Alexandre Belloni wrote:
> On 17/05/2024 17:16:58-0700, Kees Cook wrote:
> > Argument processing is specific to the test harness code. Any optional
> > information needs to be passed via environment variables. Move alternate
> >
or is
> enabled for test builds.
>
> Rearrange arithmetic and use check_add_overflow() for validating the
> allocation size to avoid the overflow.
>
> Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
> subsystem")
> Cc: Javier Martinez Canillas
>
or is
> enabled for test builds.
>
> Rearrange arithmetic and use check_add_overflow() for validating the
> allocation size to avoid the overflow.
>
> Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the
> subsystem")
> Cc: Javier Martinez Canillas
>
When memcmp() returns a non-zero value, only the signed bit has any
meaning. The actual value may differ between implementations.
Reported-by: Nathan Chancellor
Closes: https://github.com/ClangBuiltLinux/linux/issues/2025
Tested-by: Nathan Chancellor
Signed-off-by: Kees Cook
---
Cc: linux
aling()
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> [1]
> Link: https://github.com/KSPP/linux/issues/160 [2]
> Signed-off-by: Christophe JAILLET
Yes please! :)
Reviewed-by: Kees Cook
--
Kees Cook
_host_cmd
> *cmd)
> out_meta->callback = cmd->callback;
>
> out_cmd->hdr.cmd = cmd->id;
> - memcpy(_cmd->cmd.payload, cmd->data, cmd->len);
> + memcpy(_cmd->hdr.data, cmd->data, cmd->len);
>
> /* At this point, the out_cmd now has all of the incoming cmd
>* information */
--
Kees Cook
ter yet, since "sizeof(*args) + size" is repeated 3 times in the
function, I'd recommend:
...
u32 args_size;
if (check_add_overflow(sizeof(*args), size, _size))
return -ENOMEM;
if (args_size > sizeof(stack)) {
if (!(args = kmalloc(args_size, GFP_KERNEL)))
return -ENOMEM;
} else {
args = (void *)stack;
}
...
ret = nvif_object_ioctl(object, args, args_size, NULL);
This will catch the u32 overflow to nvif_object_ioctl(), catch an
allocation underflow on 32-bits systems, and make the code more
readable. :)
-Kees
--
Kees Cook
ter yet, since "sizeof(*args) + size" is repeated 3 times in the
function, I'd recommend:
...
u32 args_size;
if (check_add_overflow(sizeof(*args), size, _size))
return -ENOMEM;
if (args_size > sizeof(stack)) {
if (!(args = kmalloc(args_size, GFP_KERNEL)))
return -ENOMEM;
} else {
args = (void *)stack;
}
...
ret = nvif_object_ioctl(object, args, args_size, NULL);
This will catch the u32 overflow to nvif_object_ioctl(), catch an
allocation underflow on 32-bits systems, and make the code more
readable. :)
-Kees
--
Kees Cook
I'd still like to replace
all the open-coded TEST_HARNESS_MAIN calls, though.
--
Kees Cook
Instead of mixing selftest harness and ksft helpers, perform SKIP
testing from the FIXTURE_SETUPs. This also means TEST_HARNESS_MAIN does
not need to be open-coded.
Signed-off-by: Kees Cook
---
Cc: Christian Borntraeger
Cc: Janosch Frank
Cc: Claudio Imbrenda
Cc: David Hildenbrand
Cc: Shuah
Avoid open-coding TEST_HARNESS_MAIN. (It might change, for example.)
Signed-off-by: Kees Cook
---
Cc: Jiri Kosina
Cc: Benjamin Tissoires
Cc: Shuah Khan
Cc: Masahiro Yamada
Cc: linux-in...@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
---
tools/testing/selftests/hid/hid_bpf.c | 12
be done in the FIXTURE_SETUP(). With
this adjustment, also improve the error reporting when the device cannot
be opened.
Signed-off-by: Kees Cook
---
Cc: Alexandre Belloni
Cc: Shuah Khan
Cc: Masahiro Yamada
Cc: linux-...@vger.kernel.org
Cc: linux-kselftest@vger.kernel.org
---
tools/testing
gt; }
>
> -static void __attribute__((constructor))
> -__constructor_order_last(void)
> -{
> - if (!__constructor_order)
> - __constructor_order = _CONSTRUCTOR_ORDER_BACKWARD;
> -}
> -
> int main(int argc, char **argv)
> {
> switch (argc) {
A better question is why these tests are open-coding the execution of
"main"...
--
Kees Cook
ys got 2.
IIRC, and it was a long time ago now, it was actually a difference
between libc implementations where I encountered the problem. Maybe
glibc vs Bionic?
-Kees
--
Kees Cook
palatable way to leave the behavior unchanged while gaining
compiler coverage.
-Kees
[1] https://lore.kernel.org/linux-hardening/202405171329.019F2F566C@keescook/
--
Kees Cook
for the compiler, though.
I think a great example recently was Peter's work creating "cleanup.h".
This feature totally changes how people have to read a function (increase
in burden), leans heavily on compiler behaviors, and shifts the burden
of correctness away from the human. It's great! But it's also _very
different_ from traditional/old-school C.
-Kees
--
Kees Cook
moot once the issue is fixed properly but reduces the disruption
> while that happens.
>
> Signed-off-by: Mark Brown
Reviewed-by: Kees Cook
--
Kees Cook
uld inform us what further steps
down this path can look like.
-Kees
--
Kees Cook
rojects for
> older kernel releases. Would such an extension be useful to show new
> defects in addition to the current release testing?
The only one we (lightly) manage right now is the linux-next scanner. If
other folks want to host scanners for -stable kernels, that would be
interesting, yes.
-Kees
--
Kees Cook
ves
>the UB from signed overflow by mandating 2s complement.
I am a broken record. :) This is _not_ about undefined behavior.
This is about finding a way to make the intent of C authors unambiguous. That
overflow wraps is well defined. It is not always _desired_. C has no way to
distinguish between the two cases.
-Kees
--
Kees Cook
...@kernel.org
Fixes: 918327e9b7ff ("ubsan: Remove CONFIG_UBSAN_SANITIZE_ALL")
Signed-off-by: Kees Cook
---
Cc: Marco Elver
Cc: Andrey Konovalov
Cc: Andrey Ryabinin
Cc: kasan-...@googlegroups.com
Cc: linux-hardening@vger.kernel.org
---
lib/Kconfig.ubsan | 1 +
1 file changed, 1
https://github.com/kees approved this pull request.
Thanks for the updates! Let's get this in and continue with the rest of the
support. :)
https://github.com/llvm/llvm-project/pull/90786
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
ed-off-by: Masahiro Yamada
Oh, er, we probably want the reverse of this: add a depends to
CONFIG_UBSAN, otherwise we may end up trying to build with UBSAN when it
is not supported. That was my intention -- it looks like I missed adding
the line. :(
-Kees
--
Kees Cook
gt; of the total lack of Reviewed-by:s and Acked-by:s.
Oh, I thought I had already reviewed it. FWIW, please consider it:
Reviewed-by: Kees Cook
> The code appears to be stable enough for a merge.
Agreed.
> It's awkward that we're in conference this week, but I ask people to
> give con
t influence the value range
tracking? That continues to look like the root cause for these things.
-Kees
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108306
--
Kees Cook
gzilla/show_bug.cgi?id=108306
the difference being operating on an enum. I will reduce the case
and open a bug report for it.
- 1: still examining. It looks like a false positive so far.
Thanks!
-Kees
--
Kees Cook
UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE
> (for strcpy/memcpy-family functions).
>
> Cc: Mikulas Patocka
> Cc: Kees Cook
> Cc: "Gustavo A. R. Silva"
> Cc: Nathan Chancellor
> Cc: Nick Desaulniers
> Cc: Justin Stitt
> Cc: linux-ker...@vger.kern
On Tue, May 14, 2024 at 01:38:49AM +0200, Andrew Pinski wrote:
> On Mon, May 13, 2024, 11:41 PM Kees Cook wrote:
> > But it makes no sense to warn about:
> >
> > void sparx5_set (int * ptr, struct nums * sg, int index)
> > {
> >if (index >= 4)
> >
On Mon, May 13, 2024 at 10:06:59PM +, Justin Stitt wrote:
> On Mon, May 13, 2024 at 01:01:57PM -0700, Kees Cook wrote:
> > On Thu, May 09, 2024 at 11:42:07PM +, Justin Stitt wrote:
> > > fs/read_write.c | 18 +++---
> > > fs/remap_range.c | 12
On Tue, May 14, 2024 at 07:39:31AM +0900, Masahiro Yamada wrote:
> On Tue, May 14, 2024 at 3:48 AM Kees Cook wrote:
> > I am worried about the use of "guess" and "most", though. :) Before, we
> > had some clear opt-out situations, and now it's more of a si
; statements is the range tracking [4,INT_MAX].)
However, in the case where jump threading has split the execution flow
and produced a copy of "*val = sg->vals[index];" where the value range
tracking for "index" is now [4,INT_MAX], is the warning valid. But it
is only for that instance. Reporting it for effectively both (there is
only 1 source line for the array indexing) is misleading because there
is nothing the user can do about it -- the compiler created the copy and
then noticed it had a range it could apply to that array index.
This situation makes -Warray-bounds unusable for the Linux kernel (we
cannot have false positives says BDFL), but we'd *really* like to have
it enabled since it usually finds real bugs. But these false positives
can't be fixed on our end. :( So, moving them to -Warray-bounds=2 makes
sense as that's the level documented to have potential false positives.
-Kees
--
Kees Cook
scalls.sh
> VDSOarch/x86/um/vdso/vdso.so.dbg
> arch/x86/um/vdso/vdso.so.dbg: undefined symbols found
>
> Signed-off-by: Roberto Sassu
This is fixed differently (and more universally) here:
https://lore.kernel.org/lkml/20240506133544.2861555-1-masahi...@kernel.org/
--
Kees Cook
@@ -44,17 +44,17 @@ static int generic_remap_checks(struct file *file_in,
> loff_t pos_in,
> if (!IS_ALIGNED(pos_in, bs) || !IS_ALIGNED(pos_out, bs))
> return -EINVAL;
>
> - /* Ensure offsets don't wrap. */
> - if (pos_in + count < pos_in || pos_out + count < pos_out)
> - return -EINVAL;
> + if (check_add_overflow(pos_in, count, _sum) ||
> + check_add_overflow(pos_out, count, _sum))
> + return -EOVERFLOW;
Yeah, this is a good error code change. This is ultimately exposed via
copy_file_range, where this error is documented as possible.
-Kees
--
Kees Cook
, the code is more safer.
>
> This code was detected with the help of Coccinelle, and audited and
> modified manually.
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> [1]
> Link: https://github.com/KSPP/linux/issues/160 [2]
> Signed-off-by: Erick Archer
Thanks!
Reviewed-by: Kees Cook
--
Kees Cook
2)'
> 2251 static ssize_t
> 2252 store_temp_offset(struct device *dev, struct device_attribute *attr,
> 2253const char *buf, size_t count)
> 2254 {
> 2255 struct nct6775_data *data = dev_get_drvdata(dev);
> 2256 struct sensor_device_attribute *sattr =
> to_sensor_dev_attr(attr);
> 2257 int nr = sattr->index;
> 2258 long val;
> 2259 int err;
> 2260
> 2261 err = kstrtol(buf, 10, );
> 2262 if (err < 0)
> 2263 return err;
> 2264
> 2265 val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -128, 127);
> ^^
> Overflow and then clamp.
Looks like DIV_ROUND_CLOSEST() needs to be made sane and use more modern
helpers (it appears to be doing open-coded is_signed(), wants
check_*_overflow(), etc).
>
> 2266
> 2267 mutex_lock(>update_lock);
>
> regards,
> dan carpenter
-Kees
--
Kees Cook
ccinelle, and audited and
> modified manually.
>
> Link:
> https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays
> [1]
> Link:
> https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arg
n coded
> > > arithmetic". It should have been in a separate patch.
> >
> > +1, please split these changes into its own patch so we can apply it
> > separately.
>
> Ok, no problem. Also, I will simplify the "bacpy" lines with direct
> assignments as Kees suggested:
>
>di[n].src = dev->src;
>di[n].dst = dev->dst;
>
> instead of:
>
>bacpy([n].src, >src);
>bacpy([n].dst, >dst);
I think that's a separate thing and you can leave bacpy() as-is for now.
--
Kees Cook
ude arch/x86/entry/vdso/vma.o into GCOV, KCOV
> - include arch/x86/um/vdso/vma.o into KASAN, GCOV, KCOV
I would agree that these cases are all likely desirable.
Did you find any cases where you found that instrumentation was _removed_
where not expected?
-Kees
--
Kees Cook
es all the
types by examining the arguments and avoids repeating the destination
argument. So this would become:
wrapping_assign_add(sum, buf[i]);
Still not as clean as "+=", but at least more readable than the
alternatives and leaves no question about wrapping intent.
-Kees
[1] https://lore.kernel.org/lkml/202405081949.0565810E46@keescook/
--
Kees Cook
his way, the code is more readable, idiomatic and safer.
>
> This code was detected with the help of Coccinelle, and audited and
> modified manually.
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> [1]
> Link: https://github.com/KSPP/linux/issues/160 [2]
>
> Signed-off-by: Erick Archer
Looks right to me. Thanks!
Reviewed-by: Kees Cook
--
Kees Cook
more readable, idiomatic and safer.
>
> This code was detected with the help of Coccinelle, and audited and
> modified manually.
>
> Link:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> [1]
> Link: h
On Sun, May 12, 2024 at 09:32:40PM +0200, Joel Granados wrote:
> On Sat, May 11, 2024 at 11:51:18AM +0200, Thomas Weißschuh wrote:
> > Hi Kees,
> >
> > On 2024-05-08 10:11:35+, Kees Cook wrote:
> > > On Wed, Apr 24, 2024 at 08:12:34PM -0700, Jakub Kicinski wrot
On Sun, May 12, 2024 at 09:32:40PM +0200, Joel Granados wrote:
> On Sat, May 11, 2024 at 11:51:18AM +0200, Thomas Weißschuh wrote:
> > Hi Kees,
> >
> > On 2024-05-08 10:11:35+, Kees Cook wrote:
> > > On Wed, Apr 24, 2024 at 08:12:34PM -0700, Jakub Kicinski wrot
On Sun, May 12, 2024 at 09:32:40PM +0200, Joel Granados wrote:
> On Sat, May 11, 2024 at 11:51:18AM +0200, Thomas Weißschuh wrote:
> > Hi Kees,
> >
> > On 2024-05-08 10:11:35+, Kees Cook wrote:
> > > On Wed, Apr 24, 2024 at 08:12:34PM -0700, Jakub Kicinski wrot
@@ -0,0 +1,187 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#define __counted_by(f) __attribute__((counted_by(f)))
+
+struct bar;
+
+struct not_found {
+ int count;
+ struct bar *fam[] __counted_by(bork); // expected-error {{use of undeclared
identifier 'bork'}}
+};
+
kees wrote:
> Consider this example. It tries to illustrate why putting `__counted_by()` on
> a pointer to a structs containing flexible array members doesn't make sense.
>
> ```c
> struct HasFAM {
> int count;
> char buffer[] __counted_by(count); // This is OK
> };
>
> struct
Hi,
>
> This patch can be considered v4 of this other one [1]. However, since
> the patch has completely changed due to the addition of the flex array,
> I have decided to make a new series and remove the "Reviewed-by:" tag
> by Gustavo A. R. Silva and Kees cook.
>
>
ed strncpy with strscpy_pad
Kees Cook (21):
string: Prepare to merge strscpy_kunit.c into string_kunit.c
string: Merge strscpy KUnit tests into string_kunit.c
string: Prepare to merge strcat KUnit tests into string_kunit.c
string: Merge strcat KUnit tests into string_kunit.c
strin
kees wrote:
> As @apple-fcloutier @rapidsna noted this is how `-fbounds-safety` is
> currently implemented (because its much simpler) but it is a restriction that
> could be lifted in future by only requiring `struct bar` to be defined at the
> point that `foo::bar` is used rather than when
On Fri, May 10, 2024 at 12:07:12AM +, Edward Liaw wrote:
> _GNU_SOURCE is provided by lib.mk, so it should be dropped to prevent
> redefinition warnings.
>
> Reviewed-by: John Hubbard
> Reviewed-by: Muhammad Usama Anjum
> Signed-off-by: Edward Liaw
Acked-by: Kees Cook
--
Kees Cook
On Fri, May 10, 2024 at 12:06:30AM +, Edward Liaw wrote:
> -D_GNU_SOURCE can be de-duplicated here, as it is added by lib.mk.
>
> Signed-off-by: Edward Liaw
Thanks!
Acked-by: Kees Cook
--
Kees Cook
compiler version test in the
Makefile to exclude the static pie tests.
There's also the potential issue with arm64 builds that caused the
original attempt at -static. We'll likely need an exclusion there too.
--
Kees Cook
y harmless
overflows), and then dig into implicit integer truncation next.
Thank you for spending the time to look at all of this. It's a big topic
that many people feel very strongly about, so it's good to get some
"approved" direction on it. :)
-Kees
[1] These are NOT meant to be upstreamed as-is. We have idiom exclusions
to deal with, etc, but here's the tree I haven't refreshed yet:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=dev/v6.8-rc2/signed-trunc-sanitizer
[2] c14679d7005a ("wifi: cfg80211: Annotate struct cfg80211_mbssid_elems with
__counted_by")
--
Kees Cook
gain this
correctness coverage? It's accepted that there will be a non-trivial
impact, and I agree we can start to consider how to optimize
implementations. But for now I'd like to solve for how to even represent
arithmetic overflow intent at the source level.
-Kees
--
Kees Cook
On Wed, May 08, 2024 at 01:07:38PM -0700, Linus Torvalds wrote:
> On Wed, 8 May 2024 at 12:45, Kees Cook wrote:
> >
> > On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote:
> > > Example:
> > >
> > >static inline u32 __hash_32_generic(u3
On Wed, May 08, 2024 at 10:52:44AM -0700, Linus Torvalds wrote:
> On Tue, 7 May 2024 at 16:27, Kees Cook wrote:
> [...]
> Put another way the absolute first and fundamental thing you should
> look at is to make sure tools don't complain about sane behavior.
Agreed, and I expli
rs here but I did not
> want to change more than what was necessary.
>
> Link: https://github.com/llvm/llvm-project/pull/82432 [1]
> Closes: https://github.com/KSPP/linux/issues/357
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt
Yeah, this looks good. Thanks!
Reviewed-by: Kees Cook
--
Kees Cook
ELF alignment). This does, however, have the benefit of being able to
use MAP_FIXED_NOREPLACE, to avoid potential collisions.
With this fixed, enable the static PIE self tests again.
Reported-by: H.J. Lu
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=215275
Signed-off-by: Kees Cook
---
Cc: H.J. L
separately, this has no behavioral impact.
Signed-off-by: Kees Cook
---
fs/binfmt_elf.c | 52 +
1 file changed, 27 insertions(+), 25 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5397b552fbeb..56432e019d4e 100644
--- a/fs/binfmt_elf.
months. :)
Thanks!
-Kees
Kees Cook (3):
selftests/exec: Build both static and non-static load_address tests
binfmt_elf: Calculate total_size earlier
binfmt_elf: Honor PT_LOAD alignment for static PIE
fs/binfmt_elf.c | 94 ++---
tools/testing
://bugzilla.kernel.org/show_bug.cgi?id=215275 [1]
Signed-off-by: Kees Cook
---
Cc: Chris Kennelly
Cc: Eric Biederman
Cc: Shuah Khan
Cc: Muhammad Usama Anjum
Cc: John Hubbard
Cc: Fangrui Song
Cc: Andrew Morton
Cc: Yang Yingliang
Cc: linux...@kvack.org
Cc: linux-kselftest@vger.kernel.org
---
to
.
Suggested-by: "Thomas Weißschuh"
Link:
https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-11-e0beccb83...@weissschuh.net/
[1]
Signed-off-by: Kees Cook
---
Cc: "Thomas Weißschuh"
Cc: Joel Granados
Cc: Luis Chamberlain
Cc: Andy Lutomirski
Cc: Will Drewry
---
10 go via their respective subsystems, and once all
of those are in Linus's tree, send patch 11 as a stand-alone PR.
(From patch 11, it looks like the seccomp read/write function changes
could be split out? I'll do that now...)
-Kees
--
Kees Cook
___
10 go via their respective subsystems, and once all
of those are in Linus's tree, send patch 11 as a stand-alone PR.
(From patch 11, it looks like the seccomp read/write function changes
could be split out? I'll do that now...)
-Kees
--
Kees Cook
10 go via their respective subsystems, and once all
of those are in Linus's tree, send patch 11 as a stand-alone PR.
(From patch 11, it looks like the seccomp read/write function changes
could be split out? I'll do that now...)
-Kees
--
Kees Cook
goto end_coredump;
>
> - /* For cell spufs */
> + /* For cell spufs and x86 xstate */
> if (elf_coredump_extra_notes_write(cprm))
> goto end_coredump;
>
> diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
> index b54b313bcf07..e30a9b47dc87 100644
> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -411,6 +411,7 @@ typedef struct elf64_shdr {
> #define NT_X86_XSTATE0x202 /* x86 extended state using
> xsave */
> /* Old binutils treats 0x203 as a CET state */
> #define NT_X86_SHSTK 0x204 /* x86 SHSTK state */
> +#define NT_X86_XSAVE_LAYOUT 0x205 /* XSAVE layout description */
> #define NT_S390_HIGH_GPRS0x300 /* s390 upper register halves */
> #define NT_S390_TIMER0x301 /* s390 timer register */
> #define NT_S390_TODCMP 0x302 /* s390 TOD clock comparator
> register */
> --
> 2.34.1
>
Otherwise looks good. I'd like to see feedback from Intel folks too.
Thanks for working on this!
-Kees
--
Kees Cook
for __counted_by_{le, be}
https://git.kernel.org/kees/c/6d305cbef1aa
Take care,
--
Kees Cook
...
return cdi->ops->select_speed(cdi, arg);
}
And the arg there is unsigned long (?!).
So I think probably the prototype should be changed to unsigned long,
and then it can clamp the "speed" argument:
+ /* avoid exceeding the max speed or overflowing integer bounds */
; sum for the following check.
>
> Link: https://github.com/llvm/llvm-project/pull/82432 [1]
> Closes: https://github.com/KSPP/linux/issues/356
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt
I think this makes the checking more reading too. Thanks
Reviewed-by: Kees Cook
--
Kees Cook
/linux/issues/354
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Justin Stitt
Much more idiomatic. :)
Reviewed-by: Kees Cook
--
Kees Cook
_GNU_SOURCE
It's a lot of churn, but I don't see a way to do it differently. :)
Reviewed-by: Kees Cook
--
Kees Cook
rk. Many coverage areas are excluded, but the scope of what's
needed is observable. Notably, this has not been updated to use the
"wraps" attribute, which should make refactoring much less ugly:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=wip/v6.9-rc2/unsigned-overflow-sanitizer
[9]
https://lore.kernel.org/all/9162660e-2d6b-47a3-bfa2-77bfc55c817b@paulmck-laptop/
[10] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3201.pdf
--
Kees Cook
};
struct mid_flex { struct flex_hdr hdr; int n; int o; };
Then struct flex member names don't have to change, but if anything is
trying to get at struct flex::data through struct mid_flex::hdr, that'll
need casting. But it _shouldn't_ since it has "n" and "o".
-Kees
[1] https://gcc.gnu.org/pipermail/gcc-patches/2023-May/620122.html
[2] https://github.com/RTEMS/rtems
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h#n10
[4] https://git.kernel.org/linus/896880ff30866f386ebed14ab81ce1ad3710cfc4
[5]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/stddef.h?h=v6.8#n11
--
Kees Cook
On Mon, May 06, 2024 at 04:30:27PM -0700, Fangrui Song wrote:
> On Tue, Apr 16, 2024 at 10:28 AM Kees Cook wrote:
> >
> > On Tue, Apr 16, 2024 at 08:28:29PM +0500, Muhammad Usama Anjum wrote:
> > > The -static overrides the -pie and binaries aren't position independent
&g
; Also, the runtime results are the same for both clang and gcc builds.
Thanks for this! It got solved differently here:
https://lkml.kernel.org/r/20240416152831.319-1-usama.an...@collabora.com
Does that work for you as well?
-Kees
--
Kees Cook
into the
coming v6.10 merge window is important. :)
-Kees
--
Kees Cook
the patch series).
>
> From my perspective, it is for you to decide - but of course, other
> maintainers might have a different opinion about it.
>
> > #endif /* _UAPI_LINUX_STDDEF_H */
> >
> > If this is the right path, can these changes be merged into a
&
compiler_types:
add Endianness-dependent __counted_by_{le,be}").
> If this is the right path, can these changes be merged into a
> single patch or is it better to add a previous patch to define
> __counted_by{le,be}?
We're almost on top of the merge window, so how about this: send me a
patch for just the UAPI addition, and I'll include it in this coming (next
week expected) merge window. Once -rc2 is out, re-send this batman-adv
patch since then netdev will be merged with -rc2 and the UAPI change
will be there.
-Kees
--
Kees Cook
s[0] = all_buf + PAGE_SIZE;
if (nr_pages) {
rb->nr_pages = 1;
+ rb->data_pages[0] = all_buf + PAGE_SIZE;
rb->page_order = ilog2(nr_pages);
}
Also, why does rb_alloc() take an "int" nr_pages? The only caller has an
unsigned long argument for nr_pages. Nothing checks for >INT_MAX that I
can find.
--
Kees Cook
which may happen.
Before:
vmlinux.o: warning: objtool: bad call to elf_init_reloc_text_sym() for data
symbol .rodata
After:
vmlinux.o: warning: objtool: .cfi_sites: unexpected reference to non-executable
symbol '.rodata'
Signed-off-by: Kees Cook
---
Cc: Josh Poimboeuf
Cc: Peter Zijl
On Sat, May 04, 2024 at 03:24:02PM -0700, Josh Poimboeuf wrote:
> On Tue, Apr 30, 2024 at 04:51:07PM -0700, Kees Cook wrote:
> > @@ -891,8 +892,8 @@ struct reloc *elf_init_reloc_text_sym(struct elf *elf,
> > struct section *sec,
> > int addend = insn_off;
> &g
On Sat, May 04, 2024 at 12:03:18AM +0100, Al Viro wrote:
> On Fri, May 03, 2024 at 03:46:25PM -0700, Kees Cook wrote:
> > On Fri, May 03, 2024 at 02:52:38PM -0700, Linus Torvalds wrote:
> > > That means that the file will be released - and it means that you have
> > > v
king through dma_fence_signal_timestamp_locked(), I don't see
anything about resv nor somehow looking into other fence cb_list
contents...
--
Kees Cook
On Fri, May 03, 2024 at 12:59:52PM -0700, Kees Cook wrote:
> So, yeah, I can't figure out how eventpoll_release() and epoll_wait()
> are expected to behave safely for .poll handlers.
>
> Regardless, for the simple case: it seems like it's just totally illegal
> to use get_file() in
corruption indicator that system owners using
warn_limit or panic_on_warn would like to have detected.
Link:
https://lore.kernel.org/lkml/7c41cf3c-2a71-4dbb-8f34-033789090...@gmail.com/ [1]
Suggested-by: Peter Zijlstra
Signed-off-by: Kees Cook
---
Cc: Christian Brauner
Cc: Alexander Viro
Cc: Jens Axboe
On Fri, May 03, 2024 at 01:35:09PM -0600, Jens Axboe wrote:
> On 5/3/24 1:22 PM, Kees Cook wrote:
> > On Fri, May 03, 2024 at 12:49:11PM -0600, Jens Axboe wrote:
> >> On 5/3/24 12:26 PM, Kees Cook wrote:
> >>> Thanks for doing this analysis! I suspect at leas
site to enforce the immutability of the argument
> through the callbacks prototy.
>
> [...]
Applied to for-next/hardening, thanks!
[1/1] stackleak: don't modify ctl_table argument
https://git.kernel.org/kees/c/0e148d3cca0d
Take care,
--
Kees Cook
dff0091 ("stackleak: Allow runtime disabling of kernel stack
> erasing")
> Cc: sta...@vger.kernel.org
> Acked-by: Kees Cook
> Reviewed-by: Luis Chamberlain
> Signed-off-by: Thomas Weißschuh
> ---
> This was split out of my sysctl-const-handler series [0].
>
&g
On Fri, May 03, 2024 at 12:49:11PM -0600, Jens Axboe wrote:
> On 5/3/24 12:26 PM, Kees Cook wrote:
> > Thanks for doing this analysis! I suspect at least a start of a fix
> > would be this:
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-b
On Fri, May 03, 2024 at 03:39:28PM +0200, jvoisin wrote:
> On 4/28/24 19:02, Kees Cook wrote:
> > On Sun, Apr 28, 2024 at 01:02:36PM +0200, jvoisin wrote:
> >> On 4/24/24 23:40, Kees Cook wrote:
> >>> [...]
> >>> While CONFIG_RANDOM_KMALL
1 - 100 of 37767 matches
Mail list logo
