Re: [PATCH 1/5] string.h: Introduce memtostr() and memtostr_pad()

2024-04-09 Thread Andy Shevchenko
On Wed, Apr 10, 2024 at 5:31 AM Kees Cook wrote: > > Another ambiguous use of strncpy() is to copy from strings that may not > be NUL-terminated. These cases depend on having the destination buffer > be explicitly larger than the source buffer's maximum size, having > the size of the copy exactly

[PATCH 5/5] scsi: qla2xxx: Avoid possible run-time warning with long model_num

2024-04-09 Thread Kees Cook
The prior strlcpy() replacement of strncpy() here (which was later replaced with strscpy()) expected pinfo->model_num (and pinfo->model_description) to be NUL-terminated, but it is possible it was not, as the code pattern here shows vha->hw->model_number (and vha->hw->model_desc) being exactly 1

[PATCH 4/5] scsi: mpi3mr: Avoid possible run-time warning with long manufacturer strings

2024-04-09 Thread Kees Cook
The prior use of strscpy() here expected the manufacture_reply strings to be NUL-terminated, but it is possible they are not, as the code pattern here shows, e.g., edev->vendor_id being exactly 1 character larger than manufacture_reply->vendor_id, and the strscpy() was copying only up to the size

[PATCH 2/5] scsi: mptfusion: Avoid possible run-time warning with long manufacturer strings

2024-04-09 Thread Kees Cook
The prior strscpy() replacement of strncpy() here expected the manufacture_reply strings to be NUL-terminated, but it is possible they are not, as the code pattern here shows, e.g., edev->vendor_id being exactly 1 character larger than manufacture_reply->vendor_id, and the replaced strncpy() was

[PATCH 0/5] scsi: Avoid possible run-time warning with long manufacturer strings

2024-04-09 Thread Kees Cook
Hi, Another code pattern using the gloriously ambiguous strncpy() function was turning maybe not-NUL-terminated character arrays into NUL-terminated strings. In these cases, when strncpy() is replaced with strscpy() it creates a situation where if the non-terminated string takes up the entire

[PATCH 1/5] string.h: Introduce memtostr() and memtostr_pad()

2024-04-09 Thread Kees Cook
Another ambiguous use of strncpy() is to copy from strings that may not be NUL-terminated. These cases depend on having the destination buffer be explicitly larger than the source buffer's maximum size, having the size of the copy exactly match the source buffer's maximum size, and for the

[PATCH 3/5] scsi: mpt3sas: Avoid possible run-time warning with long manufacturer strings

2024-04-09 Thread Kees Cook
The prior strscpy() replacement of strncpy() here expected the manufacture_reply strings to be NUL-terminated, but it is possible they are not, as the code pattern here shows, e.g., edev->vendor_id being exactly 1 character larger than manufacture_reply->vendor_id, and the replaced strncpy() was

Re: [PATCH] xfs: xattr: replace strncpy and check for truncation

2024-04-09 Thread Darrick J. Wong
On Tue, Apr 09, 2024 at 05:27:34PM -0700, Justin Stitt wrote: > On Tue, Apr 9, 2024 at 5:23 PM Justin Stitt wrote: > > > > Hi, > > > > On Tue, Apr 9, 2024 at 6:32 AM Christoph Hellwig wrote: > > > > > > On Fri, Apr 05, 2024 at 07:45:08PM +, Justin Stitt wrote: > > > > - memcpy(offset,

Re: [PATCH] xfs: xattr: replace strncpy and check for truncation

2024-04-09 Thread Justin Stitt
On Tue, Apr 9, 2024 at 5:23 PM Justin Stitt wrote: > > Hi, > > On Tue, Apr 9, 2024 at 6:32 AM Christoph Hellwig wrote: > > > > On Fri, Apr 05, 2024 at 07:45:08PM +, Justin Stitt wrote: > > > - memcpy(offset, prefix, prefix_len); > > > - offset += prefix_len; > > > -

Re: [PATCH] xfs: xattr: replace strncpy and check for truncation

2024-04-09 Thread Justin Stitt
Hi, On Tue, Apr 9, 2024 at 6:32 AM Christoph Hellwig wrote: > > On Fri, Apr 05, 2024 at 07:45:08PM +, Justin Stitt wrote: > > - memcpy(offset, prefix, prefix_len); > > - offset += prefix_len; > > - strncpy(offset, (char *)name, namelen); /* real name > > */ > > -

RE: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Luck, Tony
> Do ECC servers wipe their RAM by default? I know that if you build with > CONFIG_RESET_ATTACK_MITIGATION=y on an EFI system that supports the > MemoryOverwriteRequestControl EFI variable you'll get a RAM wipe... I know that after I've been running RAS tests that inject ECC errors into

Re: [POC][RFC][PATCH 1/2] mm/x86: Add wildcard * option as memmap=nn*align:name

2024-04-09 Thread Kees Cook
On Tue, Apr 09, 2024 at 07:11:56PM -0400, Steven Rostedt wrote: > On Tue, 9 Apr 2024 15:23:07 -0700 > Kees Cook wrote: > > > Do we need to involve e820 at all? I think it might be possible to just > > have pstore call request_mem_region() very early? Or does KASLR make > > that unstable? > >

Re: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Kees Cook
On Tue, Apr 09, 2024 at 10:25:33PM +, Luck, Tony wrote: > >> I forgot to mention that this makes it trivial for any machine that doesn't > >> clear memory on soft-reboot, to enable console ramoops (to have access to > >> the last boot dmesg without needing serial). > >> > >> I tested this on

Re: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Steven Rostedt
On Tue, 9 Apr 2024 22:25:33 + "Luck, Tony" wrote: > >> I forgot to mention that this makes it trivial for any machine that doesn't > >> clear memory on soft-reboot, to enable console ramoops (to have access to > >> the last boot dmesg without needing serial). > >> > >> I tested this on a

Re: [POC][RFC][PATCH 2/2] pstore/ramoops: Add ramoops.mem_name= command line option

2024-04-09 Thread Steven Rostedt
On Tue, 9 Apr 2024 15:18:45 -0700 Kees Cook wrote: > > @@ -914,6 +919,19 @@ static void __init ramoops_register_dummy(void) > > { > > struct ramoops_platform_data pdata; > > > > +#ifndef MODULE > > + /* Only allowed when builtin */ > > Why only when builtin? Well, because the memory

Re: [POC][RFC][PATCH 1/2] mm/x86: Add wildcard * option as memmap=nn*align:name

2024-04-09 Thread Steven Rostedt
On Tue, 9 Apr 2024 15:23:07 -0700 Kees Cook wrote: > Do we need to involve e820 at all? I think it might be possible to just > have pstore call request_mem_region() very early? Or does KASLR make > that unstable? Yeah, would that give the same physical memory each boot, and can we guarantee

Re: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Joel Fernandes
> On Apr 10, 2024, at 3:55 AM, Luck, Tony wrote: > >  >> >>> I forgot to mention that this makes it trivial for any machine that doesn't >>> clear memory on soft-reboot, to enable console ramoops (to have access to >>> the last boot dmesg without needing serial). >>> >>> I tested this on a

RE: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Luck, Tony
>> I forgot to mention that this makes it trivial for any machine that doesn't >> clear memory on soft-reboot, to enable console ramoops (to have access to >> the last boot dmesg without needing serial). >> >> I tested this on a couple of my test boxes and on QEMU, and it works rather >> well. >

Re: [POC][RFC][PATCH 1/2] mm/x86: Add wildcard * option as memmap=nn*align:name

2024-04-09 Thread Kees Cook
On Tue, Apr 09, 2024 at 05:02:55PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > In order to allow for requesting a memory region that can be used for > things like pstore on multiple machines where the memory is not the same, > add a new option to the memmap=nn$ kernel

Re: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Kees Cook
On Tue, Apr 09, 2024 at 05:23:58PM -0400, Steven Rostedt wrote: > On Tue, 09 Apr 2024 17:02:54 -0400 > Steven Rostedt wrote: > > > memmap=12M*4096:oops ramoops.mem_name=oops > > I forgot to mention that this makes it trivial for any machine that doesn't > clear memory on soft-reboot, to

Re: [POC][RFC][PATCH 2/2] pstore/ramoops: Add ramoops.mem_name= command line option

2024-04-09 Thread Kees Cook
On Tue, Apr 09, 2024 at 05:02:56PM -0400, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > Add a method to find a region specified by memmap=nn*align:name for > ramoops. Adding a kernel command line parameter: > > memmap=12M*4096:oops ramoops.mem_name=oops > > Will use the size

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

2024-04-09 Thread Jakub Kicinski
On Tue, 9 Apr 2024 18:01:40 +0100 Edward Cree wrote: > > Shared branch would be good. Ed has some outstanding patches > > to refactor the ethtool RSS API. > > For the record I am extremely unlikely to have time to get those > done this cycle :( > Though in any case fwiw it doesn't look like

Re: [POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Steven Rostedt
On Tue, 09 Apr 2024 17:02:54 -0400 Steven Rostedt wrote: > memmap=12M*4096:oops ramoops.mem_name=oops I forgot to mention that this makes it trivial for any machine that doesn't clear memory on soft-reboot, to enable console ramoops (to have access to the last boot dmesg without needing

[POC][RFC][PATCH 1/2] mm/x86: Add wildcard * option as memmap=nn*align:name

2024-04-09 Thread Steven Rostedt
From: "Steven Rostedt (Google)" In order to allow for requesting a memory region that can be used for things like pstore on multiple machines where the memory is not the same, add a new option to the memmap=nn$ kernel command line. The memmap=nn$addr will reserve nn amount of memory at the

[POC][RFC][PATCH 0/2] pstore/mm/x86: Add wildcard memmap to map pstore consistently

2024-04-09 Thread Steven Rostedt
Add wildcard option of reserving physical memory on kernel command line Background: In ChromeOS, we have 1 MB of pstore ramoops reserved so that we can extract dmesg output and some other information when a crash happens in the field. (This is only done when the user selects "Allow Google to

[POC][RFC][PATCH 2/2] pstore/ramoops: Add ramoops.mem_name= command line option

2024-04-09 Thread Steven Rostedt
From: "Steven Rostedt (Google)" Add a method to find a region specified by memmap=nn*align:name for ramoops. Adding a kernel command line parameter: memmap=12M*4096:oops ramoops.mem_name=oops Will use the size and location defined by the memmap parameter where it finds the memory and labels

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

2024-04-09 Thread Edward Cree
On 09/04/2024 02:36, Jakub Kicinski wrote: > On Mon, 8 Apr 2024 14:07:30 +0300 Leon Romanovsky wrote: >> Jakub, do you want shared branch for this series or should I take >> everything through RDMA tree as netdev part is small enough? > > Shared branch would be good. Ed has some outstanding

Re: [PATCH] xfs: replace deprecated strncpy with strscpy_pad

2024-04-09 Thread Kees Cook
On Fri, Apr 05, 2024 at 07:52:27PM +, Justin Stitt wrote: > strncpy() is deprecated for use on NUL-terminated destination strings > [1] and as such we should prefer more robust and less ambiguous string > interfaces. > > The current code has taken care of NUL-termination by memset()'ing >

Re: [PATCH] xfs: xattr: replace strncpy and check for truncation

2024-04-09 Thread Christoph Hellwig
On Fri, Apr 05, 2024 at 07:45:08PM +, Justin Stitt wrote: > - memcpy(offset, prefix, prefix_len); > - offset += prefix_len; > - strncpy(offset, (char *)name, namelen); /* real name */ > - offset += namelen; > - *offset = '\0'; > + > + combined_len =

Re: [PATCH v3 0/3] RDMA/mana_ib: Add flex array to struct mana_cfg_rx_steer_req_v2

2024-04-09 Thread Leon Romanovsky
On Mon, Apr 08, 2024 at 06:36:57PM -0700, Jakub Kicinski wrote: > On Mon, 8 Apr 2024 14:07:30 +0300 Leon Romanovsky wrote: > > Jakub, do you want shared branch for this series or should I take > > everything through RDMA tree as netdev part is small enough? > > Shared branch would be good. Ed has

Re: [PATCH v3] net: dsa: lan9303: use ethtool_puts() for lan9303_get_strings()

2024-04-09 Thread Alexander Lobakin
From: Justin Stitt Date: Mon, 08 Apr 2024 21:01:57 + > This pattern of strncpy with some pointer arithmetic setting fixed-sized > intervals with string literal data is a bit weird so let's use > ethtool_puts() as this has more obvious behavior and is less-error > prone. > > Nicely, we also

Re: [PATCH] mtd: maps: sa1100-flash: Prefer struct_size over open coded arithmetic

2024-04-09 Thread Miquel Raynal
On Sat, 2024-03-30 at 17:55:35 UTC, Erick Archer wrote: > This is an effort to get rid of all multiplications from allocation > functions in order to prevent integer overflows [1][2]. > > As the "info" variable is a pointer to "struct sa_info" and this > structure ends in a flexible array: > >