Re: [PATCH v3] fs: fix unintentional arithmetic wraparound in offset calculation
On Fri, May 17, 2024 at 02:26:47AM +0100, Al Viro wrote: > On Fri, May 17, 2024 at 02:13:22AM +0100, Matthew Wilcox wrote: > > On Fri, May 17, 2024 at 12:29:06AM +, Justin Stitt wrote: > > > When running syzkaller with the newly reintroduced signed integer > > > overflow sanitizer we encounter this report: > > > > why do you keep saying it's unintentional? it's clearly intended. > > Because they are short on actual bugs to be found by their tooling > and attempt to inflate the sound/noise rate; therefore, every time > when overflow _IS_ handled correctly, it must have been an accident - > we couldn't have possibly done the analysis correctly. And if somebody > insists that they _are_ capable of basic math, they must be dishonest. > So... "unintentional" it's going to be. > > Math is hard, mmkay? > > Al, more than slightly annoyed by that aspect of the entire thing... Yes, some of the patches I've seen floating past actually seem nice, but the vast majority just seem like make-work. And the tone is definitely inappropriate.
Re: [PATCH v3] fs: fix unintentional arithmetic wraparound in offset calculation
On Fri, May 17, 2024 at 02:13:22AM +0100, Matthew Wilcox wrote: > On Fri, May 17, 2024 at 12:29:06AM +, Justin Stitt wrote: > > When running syzkaller with the newly reintroduced signed integer > > overflow sanitizer we encounter this report: > > why do you keep saying it's unintentional? it's clearly intended. Because they are short on actual bugs to be found by their tooling and attempt to inflate the sound/noise rate; therefore, every time when overflow _IS_ handled correctly, it must have been an accident - we couldn't have possibly done the analysis correctly. And if somebody insists that they _are_ capable of basic math, they must be dishonest. So... "unintentional" it's going to be. Math is hard, mmkay? Al, more than slightly annoyed by that aspect of the entire thing...
Re: [PATCH v3] fs: fix unintentional arithmetic wraparound in offset calculation
On Fri, May 17, 2024 at 12:29:06AM +, Justin Stitt wrote: > When running syzkaller with the newly reintroduced signed integer > overflow sanitizer we encounter this report: why do you keep saying it's unintentional? it's clearly intended.
[PATCH v2] ntp: safeguard against time_constant overflow case
Using syzkaller with the recently reintroduced signed integer overflow
sanitizer produces this UBSAN report:
UBSAN: signed-integer-overflow in ../kernel/time/ntp.c:738:18
9223372036854775806 + 4 cannot be represented in type 'long'
Call Trace:
dump_stack_lvl+0x93/0xd0
handle_overflow+0x171/0x1b0
__do_adjtimex+0x1236/0x1440
do_adjtimex+0x2be/0x740
__x64_sys_clock_adjtime+0x154/0x1d0
Rework the logic surrounding time_constant and how it is incremented so
that it doesn't wrap-around.
Link: https://github.com/llvm/llvm-project/pull/82432 [1]
Closes: https://github.com/KSPP/linux/issues/352
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt
---
Changes in v2:
- Adjust commit log (thanks Thomas)
- massively simplify bounds checking for time_constant
- Link to v1:
https://lore.kernel.org/r/20240506-b4-sio-ntp-c-v1-1-a01281aa0...@google.com
---
Historically, the signed integer overflow sanitizer did not work in the
kernel due to its interaction with `-fwrapv` but this has since been
changed [1] in the newest version of Clang. It was re-enabled in the
kernel with Commit 557f8c582a9ba8ab ("ubsan: Reintroduce signed overflow
sanitizer").
Here's the syzkaller reproducer:
| # {Threaded:false Repeat:false RepeatTimes:0 Procs:1 Slowdown:1 Sandbox:
| # SandboxArg:0 Leak:false NetInjection:false NetDevices:false
| # NetReset:false Cgroups:false BinfmtMisc:false CloseFDs:false KCSAN:false
| # DevlinkPCI:false NicVF:false USB:false VhciInjection:false Wifi:false
| # IEEE802154:false Sysctl:false Swap:false UseTmpDir:false
| # HandleSegv:false Repro:false Trace:false LegacyOptions:{Collide:false
| # Fault:false FaultCall:0 FaultNth:0}}
| clock_adjtime(0x0, &(0x7f000280)={0xff, 0x0, 0x0, 0x0, 0x0, 0x0,
0x7ffe})
... which was used against Kees' tree here (v6.8rc2):
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=wip/v6.9-rc2/unsigned-overflow-sanitizer
... with this config:
https://gist.github.com/JustinStitt/824976568b0f228ccbcbe49f3dee9bf4
---
kernel/time/ntp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 406dccb79c2b..d64f69e14938 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -733,8 +733,7 @@ static inline void process_adjtimex_modes(const struct
__kernel_timex *txc,
time_esterror = txc->esterror;
if (txc->modes & ADJ_TIMECONST) {
- time_constant = txc->constant;
- if (!(time_status & STA_NANO))
+ if (!(time_status & STA_NANO) && time_constant < MAXTC)
time_constant += 4;
time_constant = min(time_constant, (long)MAXTC);
time_constant = max(time_constant, 0l);
---
base-commit: 0106679839f7c69632b3b9833c3268c316c0a9fc
change-id: 20240506-b4-sio-ntp-c-c227b02c65a3
Best regards,
--
Justin Stitt
[PATCH v3] fs: fix unintentional arithmetic wraparound in offset calculation
When running syzkaller with the newly reintroduced signed integer
overflow sanitizer we encounter this report:
UBSAN: signed-integer-overflow in ../fs/read_write.c:91:10
9223372036854775807 + 4096 cannot be represented in type 'loff_t' (aka 'long
long')
Call Trace:
dump_stack_lvl+0x93/0xd0
handle_overflow+0x171/0x1b0
generic_file_llseek_size+0x35b/0x380
... amongst others:
UBSAN: signed-integer-overflow in ../fs/read_write.c:1657:12
142606336 - -9223372036854775807 cannot be represented in type 'loff_t' (aka
'long long')
...
UBSAN: signed-integer-overflow in ../fs/read_write.c:1666:11
9223372036854775807 - -9223231299366420479 cannot be represented in type
'loff_t' (aka 'long long')
Fix the accidental overflow in these position and offset calculations
by checking for negative position values, using check_add_overflow()
helpers and clamping values to expected ranges.
Link: https://github.com/llvm/llvm-project/pull/82432 [1]
Closes: https://github.com/KSPP/linux/issues/358
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt
---
Changes in v3:
- use check_add_overflow() instead of min() to keep old -EINVAL behavior
(thanks Jan)
- shorten UBSAN splat in commit log, reword commit log
- Link to v2:
https://lore.kernel.org/r/20240509-b4-sio-read_write-v2-1-018fc1e63...@google.com
Changes in v2:
- fix some more cases syzkaller found in read_write.c
- use min over min_t as the types are the same
- Link to v1:
https://lore.kernel.org/r/20240509-b4-sio-read_write-v1-1-06bec2022...@google.com
---
Historically, the signed integer overflow sanitizer did not work in the
kernel due to its interaction with `-fwrapv` but this has since been
changed [1] in the newest version of Clang. It was re-enabled in the
kernel with Commit 557f8c582a9ba8ab ("ubsan: Reintroduce signed overflow
sanitizer").
Here's the syzkaller reproducer:
| # {Threaded:false Repeat:false RepeatTimes:0 Procs:1 Slowdown:1 Sandbox:
| # SandboxArg:0 Leak:false NetInjection:false NetDevices:false
| # NetReset:false Cgroups:false BinfmtMisc:false CloseFDs:false KCSAN:false
| # DevlinkPCI:false NicVF:false USB:false VhciInjection:false Wifi:false
| # IEEE802154:false Sysctl:false Swap:false UseTmpDir:false
| # HandleSegv:false Repro:false Trace:false LegacyOptions:{Collide:false
| # Fault:false FaultCall:0 FaultNth:0}}
| r0 = openat$sysfs(0xff9c,
&(0x7f00)='/sys/kernel/address_bits', 0x0, 0x98)
| lseek(r0, 0x7fff, 0x2)
... which was used against Kees' tree here (v6.8rc2):
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=wip/v6.9-rc2/unsigned-overflow-sanitizer
... with this config:
https://gist.github.com/JustinStitt/824976568b0f228ccbcbe49f3dee9bf4
---
fs/read_write.c | 20 +---
fs/remap_range.c | 12 ++--
2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index d4c036e82b6c..8be30c8829a9 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -88,7 +88,8 @@ generic_file_llseek_size(struct file *file, loff_t offset,
int whence,
{
switch (whence) {
case SEEK_END:
- offset += eof;
+ if (check_add_overflow(offset, eof, ))
+ return -EINVAL;
break;
case SEEK_CUR:
/*
@@ -105,7 +106,9 @@ generic_file_llseek_size(struct file *file, loff_t offset,
int whence,
* like SEEK_SET.
*/
spin_lock(>f_lock);
- offset = vfs_setpos(file, file->f_pos + offset, maxsize);
+ if (check_add_overflow(offset, file->f_pos, ))
+ return -EINVAL;
+ offset = vfs_setpos(file, offset, maxsize);
spin_unlock(>f_lock);
return offset;
case SEEK_DATA:
@@ -1416,7 +1419,7 @@ static int generic_copy_file_checks(struct file *file_in,
loff_t pos_in,
struct inode *inode_in = file_inode(file_in);
struct inode *inode_out = file_inode(file_out);
uint64_t count = *req_count;
- loff_t size_in;
+ loff_t size_in, in_sum, out_sum;
int ret;
ret = generic_file_rw_checks(file_in, file_out);
@@ -1450,8 +1453,8 @@ static int generic_copy_file_checks(struct file *file_in,
loff_t pos_in,
if (IS_SWAPFILE(inode_in) || IS_SWAPFILE(inode_out))
return -ETXTBSY;
- /* Ensure offsets don't wrap. */
- if (pos_in + count < pos_in || pos_out + count < pos_out)
+ if (check_add_overflow(pos_in, count, _sum) ||
+ check_add_overflow(pos_out, count, _sum))
return -EOVERFLOW;
/* Shorten the copy to EOF */
@@ -1467,8 +1470,8 @@ static int generic_copy_file_checks(struct file *file_in,
loff_t pos_in,
/* Don't allow overlapped copying within the same file. */
if (inode_in == inode_out &&
- pos_out + count > pos_in &&
- pos_out < pos_in + count)
+
[PATCH 1/1] wifi: mac80211: Avoid address calculations via out of bounds array indexing
req->n_channels must be set before req->channels[] can be used.
Additionally, memory addresses after the "channels" array need to be
calculated from the allocation base ("request") instead of the first
"out of bounds" index of "channels" to avoid a runtime bounds check
warning.
This patch is largely influenced by the work in [1] and fixes one or
more issues reported in [2].
[ 83.964252] [ cut here ]
[ 83.964255] UBSAN: array-index-out-of-bounds in net/mac80211/scan.c:364:4
[ 83.964258] index 0 is out of range for type 'struct ieee80211_channel *[]'
[ 83.964260] CPU: 0 PID: 1695 Comm: iwd Tainted: G OT
6.8.9-gentoo-hardened1 #1
[ 83.964262] Hardware name: System76 Pangolin/Pangolin, BIOS
ARB928_V00.01_T0025ASY1_ms 04/20/2023
[ 83.964264] Call Trace:
[ 83.964267]
[ 83.964269] dump_stack_lvl+0x3f/0xc0
[ 83.964274] __ubsan_handle_out_of_bounds+0xec/0x110
[ 83.964278] ieee80211_prep_hw_scan+0x2db/0x4b0
[ 83.964281] __ieee80211_start_scan+0x601/0x990
[ 83.964284] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964287] ? cfg80211_scan+0x149/0x250
[ 83.964291] nl80211_trigger_scan+0x874/0x980
[ 83.964295] genl_family_rcv_msg_doit+0xe8/0x160
[ 83.964298] genl_rcv_msg+0x240/0x270
[ 83.964301] ? __cfi_nl80211_trigger_scan+0x10/0x10
[ 83.964302] ? __cfi_nl80211_post_doit+0x10/0x10
[ 83.964304] ? __cfi_nl80211_pre_doit+0x10/0x10
[ 83.964307] ? __cfi_genl_rcv_msg+0x10/0x10
[ 83.964309] netlink_rcv_skb+0x102/0x130
[ 83.964312] genl_rcv+0x23/0x40
[ 83.964314] netlink_unicast+0x23b/0x340
[ 83.964316] netlink_sendmsg+0x3a9/0x450
[ 83.964319] __sys_sendto+0x3ae/0x3c0
[ 83.964324] __x64_sys_sendto+0x21/0x40
[ 83.964326] do_syscall_64+0x90/0x150
[ 83.964329] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964331] ? syscall_exit_work+0xc2/0xf0
[ 83.964333] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964335] ? syscall_exit_to_user_mode+0x74/0xa0
[ 83.964337] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964339] ? do_syscall_64+0x9c/0x150
[ 83.964340] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964342] ? syscall_exit_to_user_mode+0x74/0xa0
[ 83.964344] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964346] ? do_syscall_64+0x9c/0x150
[ 83.964347] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964349] ? do_syscall_64+0x9c/0x150
[ 83.964351] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964353] ? syscall_exit_work+0xc2/0xf0
[ 83.964354] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964356] ? syscall_exit_to_user_mode+0x74/0xa0
[ 83.964358] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964359] ? do_syscall_64+0x9c/0x150
[ 83.964361] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964362] ? do_user_addr_fault+0x488/0x620
[ 83.964366] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964367] ? srso_alias_return_thunk+0x5/0xfbef5
[ 83.964369] entry_SYSCALL_64_after_hwframe+0x55/0x5d
[ 83.964372] RIP: 0033:0x6200808578d7
[ 83.964374] Code: 00 00 90 f3 0f 1e fa 41 56 55 41 89 ce 48 83 ec 28 80 3d
7b f7 0d 00 00 74 29 45 31 c9 45 31 c0 41 89 ca b8 2c 00 00 00 0f 05 <48> 3d 00
f0 ff ff 77 71 48 83 c4 28 5d 41 5e c3 66 0f 1f 84 00 00
[ 83.964375] RSP: 002b:730c4e821530 EFLAGS: 0246 ORIG_RAX:
002c
[ 83.964378] RAX: ffda RBX: 06dbc456c570 RCX: 6200808578d7
[ 83.964379] RDX: 005c RSI: 06dbc45884f0 RDI: 0004
[ 83.964381] RBP: 0004 R08: R09:
[ 83.964382] R10: R11: 0246 R12: 06dbc456c480
[ 83.964383] R13: 06dbc456c450 R14: R15: 06dbc456c610
[ 83.964386]
[ 83.964386] ---[ end trace ]---
[ 84.232857] [ cut here ]
[ 84.232863] UBSAN: array-index-out-of-bounds in net/wireless/scan.c:893:12
[ 84.232868] index 59 is out of range for type 'struct ieee80211_channel *[]'
[ 84.232870] CPU: 11 PID: 857 Comm: kworker/u32:37 Tainted: G OT
6.8.9-gentoo-hardened1 #1
[ 84.232875] Hardware name: System76 Pangolin/Pangolin, BIOS
ARB928_V00.01_T0025ASY1_ms 04/20/2023
[ 84.232877] Workqueue: events_unbound cfg80211_wiphy_work
[ 84.232886] Call Trace:
[ 84.232889]
[ 84.232892] dump_stack_lvl+0x3f/0xc0
[ 84.232897] __ubsan_handle_out_of_bounds+0xec/0x110
[ 84.232902] cfg80211_scan_6ghz+0xf4d/0xf60
[ 84.232908] ? srso_alias_return_thunk+0x5/0xfbef5
[ 84.232911] ? srso_alias_return_thunk+0x5/0xfbef5
[ 84.232914] ? srso_alias_return_thunk+0x5/0xfbef5
[ 84.232917] ? srso_alias_return_thunk+0x5/0xfbef5
[ 84.232921] ___cfg80211_scan_done+0xd6/0x2c0
[ 84.232925] cfg80211_wiphy_work+0xb9/0x100
[ 84.232929] process_scheduled_works+0x1d5/0x340
[ 84.232935] worker_thread+0x214/0x2e0
[ 84.232939] ? __cfi_worker_thread+0x10/0x10
[ 84.232943] kthread+0x129/0x140
[ 84.232946] ? __cfi_kthread+0x10/0x10
[ 84.232949] ret_from_fork+0x4c/0x60
[ 84.232953] ?
Re: [PATCH] ntp: remove accidental integer wrap-around
On Thu, May 16, 2024 at 4:40 PM Justin Stitt wrote:
> Isn't this usually supplied from the user and can be some pretty
> random stuff? Are you suggesting we update
> timekeeping_validate_timex() to include a check to limit the maxerror
> field to (NTP_PHASE_LIMIT-(MAXFREQ / NSEC_PER_USEC))? It seems like we
> should handle the overflow case where it happens: in
> second_overflow().
Or, I suppose we could add a check to timekeeping_validate_timex() like:
if (txc->modes & ADJ_MAXERROR) {
if (txc->maxerror < 0 || txc->maxerror > NTP_PHASE_LIMIT)
return -EINVAL;
}
> Thanks
> Justin
Re: [PATCH] ntp: remove accidental integer wrap-around
Hi,
On Tue, May 14, 2024 at 3:38 AM Thomas Gleixner wrote:
>
> On Tue, May 07 2024 at 04:34, Justin Stitt wrote:
> > Using syzkaller alongside the newly reintroduced signed integer overflow
> > sanitizer spits out this report:
> >
> > [ 138.454979] [ cut here ]
> > [ 138.458089] UBSAN: signed-integer-overflow in ../kernel/time/ntp.c:461:16
> > [ 138.462134] 9223372036854775807 + 500 cannot be represented in type
> > 'long'
> > [ 138.466234] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> > 6.8.0-rc2-00038-gc0a509640e93-dirty #10
> > [ 138.471498] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > 1.16.3-debian-1.16.3-2 04/01/2014
> > [ 138.477110] Call Trace:
> > [ 138.478657]
> > [ 138.479964] dump_stack_lvl+0x93/0xd0
> > [ 138.482276] handle_overflow+0x171/0x1b0
> > [ 138.484699] second_overflow+0x2d6/0x500
> > [ 138.487133] accumulate_nsecs_to_secs+0x60/0x160
> > [ 138.489931] timekeeping_advance+0x1fe/0x890
> > [ 138.492535] update_wall_time+0x10/0x30
>
> Same comment vs. trimming.
Gotcha, in the next version this will be trimmed.
>
> > Historically, the signed integer overflow sanitizer did not work in the
> > kernel due to its interaction with `-fwrapv` but this has since been
> > changed [1] in the newest version of Clang. It was re-enabled in the
> > kernel with Commit 557f8c582a9ba8ab ("ubsan: Reintroduce signed overflow
> > sanitizer").
>
> Again. Irrelevant to the problem.
Right, I'll move it below the fold.
>
> > Let's introduce a new macro and use that against NTP_PHASE_LIMIT to
> > properly limit the max size of time_maxerror without overflowing during
> > the check itself.
>
> This fails to tell what is causing the issue and just talks about what
> the patch is doing. The latter can be seen from the patch itself, no?
>
> Something like this:
>
>On second overflow time_maxerror is unconditionally incremented and
>the result is checked against NTP_PHASE_LIMIT, but the increment can
>overflow into negative space.
>
>Prevent this by checking the overflow condition before incrementing.
>
> Hmm?
Sounds better :thumbs_up: I'll use this!
>
> But that obviously begs the question why this can happen at all.
>
> #define MAXPHASE 5L
> #define NTP_PHASE_LIMIT ((MAXPHASE / NSEC_PER_USEC) << 5)
>
> ==> NTP_PHASE_LIMIT = 1.6e+07 = 0xf42400
>
> #define MAXFREQ 50
>
> So how can 0xf42400 + 50/1000 overflow in the first place?
>
> It can't unless time_maxerror is somehow initialized to a bogus
> value and indeed it is:
>
> process_adjtimex_modes()
>
> if (txc->modes & ADJ_MAXERROR)
> time_maxerror = txc->maxerror;
>
> So that wants to be fixed and not the symptom.
Isn't this usually supplied from the user and can be some pretty
random stuff? Are you suggesting we update
timekeeping_validate_timex() to include a check to limit the maxerror
field to (NTP_PHASE_LIMIT-(MAXFREQ / NSEC_PER_USEC))? It seems like we
should handle the overflow case where it happens: in
second_overflow().
The clear intent of the existing code was to saturate at
NTP_PHASE_LIMIT, they just did it in a way where the check itself
triggers overflow sanitizers.
>
> Thanks,
>
> tglx
Thanks
Justin
Re: [RFC] Mitigating unexpected arithmetic overflow
On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote: > > It is incredibly important that the exact opposite approach is taken; > we need to be annotating (or adding type qualifiers to) the _expected_ > overflow cases. The omniscience required to go and properly annotate > all the spots that will cause problems would suggest that we should > just fix the bug outright. If only it was that easy. It certainly isn't easy, yes. But the problem is when you dump a huge amount of work and pain onto kernel developers, when they haven't signed up for it, when they don't necessarily have the time to do all of the work themselves, and when their corporate overlords won't given them the headcount to handle unfunded mandates which folks who are looking for a bright new wonderful future --- don't be surprised if kernel developers push back hard. One of the big problems that we've seen with many of these security initiatives is that the teams create these unfunded mandates get their performance reviews based on how many "bug reports" that they file, regardless of whether they are real problems or not. This has been a problem with syzkaller, and with clusterfuzz. Let's not make this problem worse with new and fancy sanitizers, please. Unfortunately, I don't get funding from my employer to clear these kinds of reports, so when I do the work, it happens on the weekends or late at night, on my own time, which is probably why I am so grumpy about this. Whether you call this "sharpening our focus", or "year of efficiency", or pick your corporate buzzwords, it really doesn't matter. The important thing is that the figure of merit must NOT be "how many security bugs that are found", but how much bullsh*t noise do these security features create, and how do you decrease overhead by upstream developers to deal with the fuzzing/ubsan/security tools find. Cheers, - Ted
Re: [RFC] Mitigating unexpected arithmetic overflow
On Thu, May 16, 2024 at 12:48:47PM -0700, Justin Stitt wrote: > I don't think we're capable of identifying every single problematic > overflow/wraparound case in the kernel, this is pretty obvious > considering we've had decades to do so. Instead, it seems much more > feasible that we annotate (very, very minimally so as not to disrupt > code readability and style) the spots where we _know_ overflow should > happen. For the baby steps Linus wants, we can walk this path: - Finish the *signed* integer overflow refactoring/annotation. This is nearly done already, and every case we've found is either a legitimate bug (thankfully rare), or happens in code that is either accidentally correct (thanks to no UB), or the correctness is very unclear. Refactoring these cases improves readability for everyone and doesn't change the behavior. - Begin *signed* integer implicit truncation refactoring/annotation. As Linus suggested, dealing with this will catch a bunch of the flaws we've seen recently. Handling the false positives here will need some investigation and some compiler support, and that'll happen in parallel. - Tackle *unsigned* integer overflow on a per-type basis: we can start with the place Linus called out: size_t. This will let us focus on the first of the unsigned types that is not commonly wrapping, and is a regular place that unexpected overflow gets the kernel into big trouble. What we learn from these three steps should inform us what further steps down this path can look like. -Kees -- Kees Cook
Re: [RFC] Mitigating unexpected arithmetic overflow
Hi, On Thu, May 16, 2024 at 7:09 AM Peter Zijlstra wrote: > > On Thu, May 16, 2024 at 06:30:32AM -0700, Kees Cook wrote: > > > > I am a broken record. :) This is _not_ about undefined behavior. > > And yet you introduced CONFIG_UBSAN_SIGNED_WRAP... *UB*san, get it? We should think of UBSAN as an "Unexpected Behavior" Sanitizer. Clang has evolved to provide instrumentation for things that are not *technically* undefined behavior. Go to [1] and grep for some phrases like "not undefined behavior" and see that we have quite a few sanitizers that aren't *technically* handling 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. > > The current semantics are (and have been for years, decades at this > point) that everything wraps nicely and code has been assuming this. You > cannot just change this. Why not :>) Lots and lots of exploits are caused by unintentional arithmetic overflow [2]. > > So what you do is do a proper language extension and add a type > qualifier that makes overflows trap and annotate all them cases where > people do not expect overflows (so that we can put the > __builtin_*_overflow() things where the sun don't shine). It is incredibly important that the exact opposite approach is taken; we need to be annotating (or adding type qualifiers to) the _expected_ overflow cases. The omniscience required to go and properly annotate all the spots that will cause problems would suggest that we should just fix the bug outright. If only it was that easy. I don't think we're capable of identifying every single problematic overflow/wraparound case in the kernel, this is pretty obvious considering we've had decades to do so. Instead, it seems much more feasible that we annotate (very, very minimally so as not to disrupt code readability and style) the spots where we _know_ overflow should happen. [1]: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#ubsan-checks [2]: https://cwe.mitre.org/data/definitions/190.html Thanks Justin
Re: Extending Linux' Coverity model and also cover aarch64
On Thu, May 16, 2024 at 03:28:16PM +, Manthey, Norbert wrote: > we published an extension for the Coverity model that is used by the > CoverityScan setup for the Linux kernel [1]. We have been using this > extension to analyze the 6.1 kernel branch, and reported some fixes to > the upstream code base that are based on this model [2]. Feel free to > merge the pull request, and update the model in the CoverityScan setup. > We do not have access to that project to perform these updates > ourselves. Thanks for this! I'll get it loaded into the Linux-Next scanner. > To increase the analysis coverage to aarch64, we analyzed a x86 and a > aarch64 configuration. The increased coverage is achieved by using re- > configuration and cross-compilation during the analysis build. If you > are interested in this setup we can share the Dockerfile and script we > used for this process. We've only got access to the free Coverity scanner, but it would be nice to see if there was anything specific to arm64. > To prevent regressions in backports to LTS kernels, we wondered whether > the community is interested in setting up CoverityScan projects 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
Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support
On Thu, May 16, 2024 at 05:25:58PM +0200, Amelie Delaunay wrote:
> On 5/15/24 20:56, Frank Li wrote:
> > On Tue, Apr 23, 2024 at 02:32:55PM +0200, Amelie Delaunay wrote:
> > > STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
> > > controller:
...
> > > + writel_relaxed(hwdesc->cdar, ddata->base + STM32_DMA3_CDAR(id));
> > > + writel_relaxed(hwdesc->cllr, ddata->base + STM32_DMA3_CLLR(id));
> > > +
> > > + /* Clear any pending interrupts */
> > > + csr = readl_relaxed(ddata->base + STM32_DMA3_CSR(id));
> > > + if (csr & CSR_ALL_F)
> > > + writel_relaxed(csr, ddata->base + STM32_DMA3_CFCR(id));
> > > +
> > > + stm32_dma3_chan_dump_reg(chan);
> > > +
> > > + ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(id));
> > > + writel_relaxed(ccr | CCR_EN, ddata->base + STM32_DMA3_CCR(id));
> >
> > This one should use writel instead of writel_relaxed because it need
> > dma_wmb() as barrier for preious write complete.
> >
> > Frank
> >
>
> ddata->base is Device memory type thanks to ioremap() use, so it is strongly
> ordered and non-cacheable.
> DMA3 is outside CPU cluster, its registers are accessible through AHB bus.
> dma_wmb() (in case of writel instead of writel_relaxed) is useless in that
> case: it won't ensure the propagation on the bus is complete, and it will
> have impacts on the system.
> That's why CCR register is written once, then it is read before CCR_EN is
> set and being written again, with _relaxed(), because registers are behind a
> bus, and ioremapped with Device memory type which ensures it is strongly
> ordered and non-cacheable.
regardless memory map, writel_relaxed() just make sure io write and read is
orderred, not necessary order with other memory access. only readl and
writel make sure order with other memory read/write.
1. Write src_addr to descriptor
2. dma_wmb()
3. Write "ready" to descriptor
4. enable channel or doorbell by write a register.
if 4 use writel_relaxe(). because 3 write to DDR, which difference place of
mmio, 4 may happen before 3. Your can refer axi order model.
4 have to use ONE writel(), to make sure 3 already write to DDR.
You need use at least one writel() to make sure all nornmal memory finish.
>
> > > +
> > > + chan->dma_status = DMA_IN_PROGRESS;
> > > +
> > > + dev_dbg(chan2dev(chan), "vchan %pK: started\n", >vchan);
> > > +}
> > > +
> > > +static int stm32_dma3_chan_suspend(struct stm32_dma3_chan *chan, bool
> > > susp)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 csr, ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id)) &
> > > ~CCR_EN;
> > > + int ret = 0;
> > > +
> > > + if (susp)
> > > + ccr |= CCR_SUSP;
> > > + else
> > > + ccr &= ~CCR_SUSP;
> > > +
> > > + writel_relaxed(ccr, ddata->base + STM32_DMA3_CCR(chan->id));
> > > +
> > > + if (susp) {
> > > + ret = readl_relaxed_poll_timeout_atomic(ddata->base +
> > > STM32_DMA3_CSR(chan->id), csr,
> > > + csr & CSR_SUSPF, 1, 10);
> > > + if (!ret)
> > > + writel_relaxed(CFCR_SUSPF, ddata->base +
> > > STM32_DMA3_CFCR(chan->id));
> > > +
> > > + stm32_dma3_chan_dump_reg(chan);
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void stm32_dma3_chan_reset(struct stm32_dma3_chan *chan)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id)) &
> > > ~CCR_EN;
> > > +
> > > + writel_relaxed(ccr |= CCR_RESET, ddata->base +
> > > STM32_DMA3_CCR(chan->id));
> > > +}
> > > +
> > > +static int stm32_dma3_chan_stop(struct stm32_dma3_chan *chan)
> > > +{
> > > + struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> > > + u32 ccr;
> > > + int ret = 0;
> > > +
> > > + chan->dma_status = DMA_COMPLETE;
> > > +
> > > + /* Disable interrupts */
> > > + ccr = readl_relaxed(ddata->base + STM32_DMA3_CCR(chan->id));
> > > + writel_relaxed(ccr & ~(CCR_ALLIE | CCR_EN), ddata->base +
> > > STM32_DMA3_CCR(chan->id));
> > > +
> > > + if (!(ccr & CCR_SUSP) && (ccr & CCR_EN)) {
> > > + /* Suspend the channel */
> > > + ret = stm32_dma3_chan_suspend(chan, true);
> > > + if (ret)
> > > + dev_warn(chan2dev(chan), "%s: timeout, data might be
> > > lost\n", __func__);
> > > + }
> > > +
> > > + /*
> > > + * Reset the channel: this causes the reset of the FIFO and the reset
> > > of the channel
> > > + * internal state, the reset of CCR_EN and CCR_SUSP bits.
> > > + */
> > > + stm32_dma3_chan_reset(chan);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static void stm32_dma3_chan_complete(struct stm32_dma3_chan *chan)
> > > +{
> > > + if (!chan->swdesc)
> > > + return;
> > > +
> > > + vchan_cookie_complete(>swdesc->vdesc);
> > > + chan->swdesc = NULL;
> > > + stm32_dma3_chan_start(chan);
> > > +}
> > > +
> > > +static irqreturn_t stm32_dma3_chan_irq(int irq, void *devid)
>
Re: Extending Linux' Coverity model and also cover aarch64
On Thu, May 16, 2024 at 03:28:16PM +, Manthey, Norbert wrote: > Dear Kees, all, > > we published an extension for the Coverity model that is used by the > CoverityScan setup for the Linux kernel [1]. We have been using this > extension to analyze the 6.1 kernel branch, and reported some fixes to > the upstream code base that are based on this model [2]. Feel free to > merge the pull request, and update the model in the CoverityScan setup. > We do not have access to that project to perform these updates > ourselves. > > To increase the analysis coverage to aarch64, we analyzed a x86 and a > aarch64 configuration. The increased coverage is achieved by using re- > configuration and cross-compilation during the analysis build. If you > are interested in this setup we can share the Dockerfile and script we > used for this process. > > To prevent regressions in backports to LTS kernels, we wondered whether > the community is interested in setting up CoverityScan projects for > older kernel releases. Would such an extension be useful to show new > defects in addition to the current release testing? New defects yes, I would like to know that, as long as they are also fixed already in mainline, right? Just send us reports of that, no need to get the covertity site involved there, I'll be glad to take them. thanks, greg k-h
Extending Linux' Coverity model and also cover aarch64
Dear Kees, all, we published an extension for the Coverity model that is used by the CoverityScan setup for the Linux kernel [1]. We have been using this extension to analyze the 6.1 kernel branch, and reported some fixes to the upstream code base that are based on this model [2]. Feel free to merge the pull request, and update the model in the CoverityScan setup. We do not have access to that project to perform these updates ourselves. To increase the analysis coverage to aarch64, we analyzed a x86 and a aarch64 configuration. The increased coverage is achieved by using re- configuration and cross-compilation during the analysis build. If you are interested in this setup we can share the Dockerfile and script we used for this process. To prevent regressions in backports to LTS kernels, we wondered whether the community is interested in setting up CoverityScan projects for older kernel releases. Would such an extension be useful to show new defects in addition to the current release testing? Best, Norbert [1] github Coverity model pull request link: https://github.com/kees/coverity-linux/pull/1 [2] Emails for most fixes by Hagar: https://lore.kernel.org/all/?q=f%3Ahagarhem Amazon Web Services Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B Sitz: Berlin Ust-ID: DE 365 538 597
Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support
On 5/15/24 20:56, Frank Li wrote: On Tue, Apr 23, 2024 at 02:32:55PM +0200, Amelie Delaunay wrote: STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3 controller: - LPDMA (Low Power): 4 channels, no FIFO - GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes - HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes Hardware configuration of the channels is retrieved from the hardware configuration registers. The client can specify its channel requirements through device tree. STM32 DMA3 channels can be individually reserved either because they are secure, or dedicated to another CPU. Indeed, channels availability depends on Resource Isolation Framework (RIF) configuration. RIF grants access to buses with Compartiment ID (CIF) filtering, secure and privilege level. It also assigns DMA channels to one or several processors. DMA channels used by Linux should be CID-filtered and statically assigned to CID1 or shared with other CPUs but using semaphore. In case CID filtering is not configured, dma-channel-mask property can be used to specify available DMA channels to the kernel, otherwise such channels will be marked as reserved and can't be used by Linux. Signed-off-by: Amelie Delaunay --- drivers/dma/stm32/Kconfig | 10 + drivers/dma/stm32/Makefile |1 + drivers/dma/stm32/stm32-dma3.c | 1431 3 files changed, 1442 insertions(+) create mode 100644 drivers/dma/stm32/stm32-dma3.c diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig index b72ae1a4502f..4d8d8063133b 100644 --- a/drivers/dma/stm32/Kconfig +++ b/drivers/dma/stm32/Kconfig @@ -34,4 +34,14 @@ config STM32_MDMA If you have a board based on STM32 SoC with such DMA controller and want to use MDMA say Y here. +config STM32_DMA3 + tristate "STMicroelectronics STM32 DMA3 support" + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Enable support for the on-chip DMA3 controller on STMicroelectronics + STM32 platforms. + If you have a board based on STM32 SoC with such DMA3 controller + and want to use DMA3, say Y here. + endif diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile index 663a3896a881..5082db4b4c1c 100644 --- a/drivers/dma/stm32/Makefile +++ b/drivers/dma/stm32/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_STM32_DMA) += stm32-dma.o obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o +obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c new file mode 100644 index ..b5493f497d06 --- /dev/null +++ b/drivers/dma/stm32/stm32-dma3.c @@ -0,0 +1,1431 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * STM32 DMA3 controller driver + * + * Copyright (C) STMicroelectronics 2024 + * Author(s): Amelie Delaunay + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../virt-dma.h" + +#define STM32_DMA3_SECCFGR 0x00 +#define STM32_DMA3_PRIVCFGR0x04 +#define STM32_DMA3_RCFGLOCKR 0x08 +#define STM32_DMA3_MISR0x0C +#define STM32_DMA3_SMISR 0x10 + +#define STM32_DMA3_CLBAR(x)(0x50 + 0x80 * (x)) +#define STM32_DMA3_CCIDCFGR(x) (0x54 + 0x80 * (x)) +#define STM32_DMA3_CSEMCR(x) (0x58 + 0x80 * (x)) +#define STM32_DMA3_CFCR(x) (0x5C + 0x80 * (x)) +#define STM32_DMA3_CSR(x) (0x60 + 0x80 * (x)) +#define STM32_DMA3_CCR(x) (0x64 + 0x80 * (x)) +#define STM32_DMA3_CTR1(x) (0x90 + 0x80 * (x)) +#define STM32_DMA3_CTR2(x) (0x94 + 0x80 * (x)) +#define STM32_DMA3_CBR1(x) (0x98 + 0x80 * (x)) +#define STM32_DMA3_CSAR(x) (0x9C + 0x80 * (x)) +#define STM32_DMA3_CDAR(x) (0xA0 + 0x80 * (x)) +#define STM32_DMA3_CLLR(x) (0xCC + 0x80 * (x)) + +#define STM32_DMA3_HWCFGR130xFC0 /* G_PER_CTRL(X) x=8..15 */ +#define STM32_DMA3_HWCFGR120xFC4 /* G_PER_CTRL(X) x=0..7 */ +#define STM32_DMA3_HWCFGR4 0xFE4 /* G_FIFO_SIZE(X) x=8..15 */ +#define STM32_DMA3_HWCFGR3 0xFE8 /* G_FIFO_SIZE(X) x=0..7 */ +#define STM32_DMA3_HWCFGR2 0xFEC /* G_MAX_REQ_ID */ +#define STM32_DMA3_HWCFGR1 0xFF0 /* G_MASTER_PORTS, G_NUM_CHANNELS, G_Mx_DATA_WIDTH */ +#define STM32_DMA3_VERR0xFF4 + +/* SECCFGR DMA secure configuration register */ +#define SECCFGR_SEC(x) BIT(x) + +/* MISR DMA non-secure/secure masked interrupt status register */ +#define MISR_MIS(x)BIT(x) + +/* CxLBAR DMA channel x linked_list base address register */ +#define CLBAR_LBA GENMASK(31, 16) + +/* CxCIDCFGR DMA channel x CID register */ +#define
Re: [RFC] Mitigating unexpected arithmetic overflow
On Thu, May 16, 2024 at 06:30:32AM -0700, Kees Cook wrote: > > > On May 15, 2024 12:36:36 AM PDT, Peter Zijlstra wrote: > >On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote: > >> For example, the most common case of overflow we've ever had has very > >> much been array indexing. Now, sometimes that has actually been actual > >> undefined behavior, because it's been overflow in signed variables, > >> and those are "easy" to find in the sense that you just say "no, can't > >> do that". UBSAN finds them, and that's good. > > > >We build with -fno-strict-overflow, which implies -fwrapv, which removes > >the UB from signed overflow by mandating 2s complement. > > I am a broken record. :) This is _not_ about undefined behavior. And yet you introduced CONFIG_UBSAN_SIGNED_WRAP... *UB*san, get it? > 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. The current semantics are (and have been for years, decades at this point) that everything wraps nicely and code has been assuming this. You cannot just change this. So what you do is do a proper language extension and add a type qualifier that makes overflows trap and annotate all them cases where people do not expect overflows (so that we can put the __builtin_*_overflow() things where the sun don't shine). And pretty please, also do a qualifier modification extension, because that's totally painful already.
Re: [RFC PATCH v3 3/5] KVM: x86: Add notifications for Heki policy configuration and violation
On Tue May 14, 2024 at 12:23 PM UTC, Mickaël Salaün wrote:
> > Development happens
> > https://github.com/vianpl/{linux,qemu,kvm-unit-tests} and the vsm-next
> > branch, but I'd advice against looking into it until we add some order
> > to the rework. Regardless, feel free to get in touch.
>
> Thanks for the update.
>
> Could we schedule a PUCK meeting to synchronize and help each other?
> What about June 12?
Sounds great! June 12th works for me.
Nicolas
Re: [RFC] Mitigating unexpected arithmetic overflow
On May 15, 2024 12:36:36 AM PDT, Peter Zijlstra wrote: >On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote: >> For example, the most common case of overflow we've ever had has very >> much been array indexing. Now, sometimes that has actually been actual >> undefined behavior, because it's been overflow in signed variables, >> and those are "easy" to find in the sense that you just say "no, can't >> do that". UBSAN finds them, and that's good. > >We build with -fno-strict-overflow, which implies -fwrapv, which removes >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
Re: [PATCH 05/12] dmaengine: Add STM32 DMA3 support
On 5/15/24 20:45, Frank Li wrote:
On Mon, May 13, 2024 at 11:21:18AM +0200, Amelie Delaunay wrote:
Hi Frank,
On 5/7/24 22:26, Frank Li wrote:
On Tue, May 07, 2024 at 01:33:31PM +0200, Amelie Delaunay wrote:
Hi Vinod,
Thanks for the review.
On 5/4/24 14:40, Vinod Koul wrote:
On 23-04-24, 14:32, Amelie Delaunay wrote:
STM32 DMA3 driver supports the 3 hardware configurations of the STM32 DMA3
controller:
- LPDMA (Low Power): 4 channels, no FIFO
- GPDMA (General Purpose): 16 channels, FIFO from 8 to 32 bytes
- HPDMA (High Performance): 16 channels, FIFO from 8 to 256 bytes
Hardware configuration of the channels is retrieved from the hardware
configuration registers.
The client can specify its channel requirements through device tree.
STM32 DMA3 channels can be individually reserved either because they are
secure, or dedicated to another CPU.
Indeed, channels availability depends on Resource Isolation Framework
(RIF) configuration. RIF grants access to buses with Compartiment ID
Compartiment? typo...?
Sorry, indeed, Compartment instead.
(CIF) filtering, secure and privilege level. It also assigns DMA channels
to one or several processors.
DMA channels used by Linux should be CID-filtered and statically assigned
to CID1 or shared with other CPUs but using semaphore. In case CID
filtering is not configured, dma-channel-mask property can be used to
specify available DMA channels to the kernel, otherwise such channels
will be marked as reserved and can't be used by Linux.
Signed-off-by: Amelie Delaunay
---
drivers/dma/stm32/Kconfig | 10 +
drivers/dma/stm32/Makefile |1 +
drivers/dma/stm32/stm32-dma3.c | 1431
3 files changed, 1442 insertions(+)
create mode 100644 drivers/dma/stm32/stm32-dma3.c
diff --git a/drivers/dma/stm32/Kconfig b/drivers/dma/stm32/Kconfig
index b72ae1a4502f..4d8d8063133b 100644
--- a/drivers/dma/stm32/Kconfig
+++ b/drivers/dma/stm32/Kconfig
@@ -34,4 +34,14 @@ config STM32_MDMA
If you have a board based on STM32 SoC with such DMA controller
and want to use MDMA say Y here.
+config STM32_DMA3
+ tristate "STMicroelectronics STM32 DMA3 support"
+ select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
+ help
+ Enable support for the on-chip DMA3 controller on STMicroelectronics
+ STM32 platforms.
+ If you have a board based on STM32 SoC with such DMA3 controller
+ and want to use DMA3, say Y here.
+
endif
diff --git a/drivers/dma/stm32/Makefile b/drivers/dma/stm32/Makefile
index 663a3896a881..5082db4b4c1c 100644
--- a/drivers/dma/stm32/Makefile
+++ b/drivers/dma/stm32/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_STM32_DMA) += stm32-dma.o
obj-$(CONFIG_STM32_DMAMUX) += stm32-dmamux.o
obj-$(CONFIG_STM32_MDMA) += stm32-mdma.o
+obj-$(CONFIG_STM32_DMA3) += stm32-dma3.o
are there any similarities in mdma/dma and dma3..?
can anything be reused...?
DMA/MDMA were originally intended for STM32 MCUs and have been used in
STM32MP1 MPUs.
New MPUs (STM32MP2, ...) and STM32 MCUs (STM32H5, STM32N6, ...) use DMA3.
Unlike DMA/MDMA, DMA3 can be declined in multiple configurations, LPDMA,
GPDMA, HPDMA, and among these global configurations, there are possible
sub-configurations (e.g. channel fifo size). stm32-dma3 uses the hardware
configuration registers to discover the controller/channels capabilities.
Reuse stm32-dma or stm32-mdma would lead to complicating the driver and
making future stm32-dma3 evolutions for next STM32 MPUs intricate and very
difficult.
I think your reason still not enough to create new driver instead try to
reuse old one.
Does register layout or dma descriptor is totally difference?
If dma descriptor format is the same, at least you can reuse prepare DMA
descriptor part.
Choose channel is independt part of DMA channel. You can create sperate
one for difference DMA engine.
Frank
stm32-dma is not considered for reuse : register layout is completely
different and this DMA controller doesn't rely on descriptors mechanism.
stm32-mdma is based on descriptors mechanism but even there, there are
significant differences in register layout and descriptors structure.
As you can see:
Can you add such description in commit message?
Frank
Ok I will enhance the commit message in new patchset version (v3).
Amelie
/* Descriptor from stm32-mdma */
struct stm32_mdma_hwdesc {
u32 ctcr;
u32 cbndtr;
u32 csar;
u32 cdar;
u32 cbrur;
u32 clar;
u32 ctbr;
u32 dummy;
u32 cmar;
u32 cmdr;
} __aligned(64);
/* Descriptor from stm32-dma3 */
struct stm32_dma3_hwdesc {
u32 ctr1;
u32 ctr2;
u32 cbr1;
u32 csar;
u32 cdar;
u32 cllr;
} __aligned(32);
Moreover, stm32-dma3 can have static or dynamic linked-list items. Dynamic
data structure support is not yet in this patchset, current implementation
is undergoing validation
Re: [RFC] Mitigating unexpected arithmetic overflow
On Wed, May 15, 2024 at 10:12:20AM -0700, Justin Stitt wrote:
> Hi Peter,
>
> On Wed, May 15, 2024 at 12:36 AM Peter Zijlstra wrote:
> >
> > On Wed, May 08, 2024 at 04:47:25PM -0700, Linus Torvalds wrote:
> > > For example, the most common case of overflow we've ever had has very
> > > much been array indexing. Now, sometimes that has actually been actual
> > > undefined behavior, because it's been overflow in signed variables,
> > > and those are "easy" to find in the sense that you just say "no, can't
> > > do that". UBSAN finds them, and that's good.
> >
> > We build with -fno-strict-overflow, which implies -fwrapv, which removes
> > the UB from signed overflow by mandating 2s complement.
>
> FWIW,
>
> Clang-19 allows -fwrapv and -fsanitize=signed-integer-overflow to work
> together [1]
>
> And the sanitizer was re-introduced with Commit 557f8c582a9ba8ab
> ("ubsan: Reintroduce signed overflow sanitizer").
Urgh, that's the exact kind of drugs we don't need. I detest that
commit. Both unsigned and signed have well defined semantics.
And since (with -fwrapv) there is no UB, UBSAN is not the right place.
> > With the exception of an UBSAN bug prior to GCC-8, UBSAN will not, and
> > should not, warn about signed overflow when using either of these flags.
>
> [1]: https://clang.llvm.org/docs/ReleaseNotes.html#sanitizers
That link doesn't seem to work for me...
