In pps_fill_timex(), use memset and offsetof instead of '= 0'.
Signed-off-by: Yejune Deng <[email protected]>
---
kernel/time/ntp.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
index 87389b9e21ab..3416c0381104 100644
--- a/kernel/time/ntp.c
+++ b/kernel/time/ntp.c
@@ -225,14 +225,11 @@ static inline int is_error_status(int status)
static inline void pps_fill_timex(struct __kernel_timex *txc)
{
/* PPS is not implemented, so these are zero */
- txc->ppsfreq = 0;
- txc->jitter = 0;
- txc->shift = 0;
- txc->stabil = 0;
- txc->jitcnt = 0;
- txc->calcnt = 0;
- txc->errcnt = 0;
- txc->stbcnt = 0;
+ int offset, len;
+
+ offset = offsetof(struct __kernel_timex, ppsfreq);
+ len = offsetof(struct __kernel_timex, tai) - offset;
+ memset(txc + offset, 0, len);
}
#endif /* CONFIG_NTP_PPS */
--
2.29.0
_______________________________________________
kbuild mailing list -- [email protected]
To unsubscribe send an email to [email protected]
Greeting,
FYI, we noticed the following commit (built with gcc-9):
commit: a29bace5d3c06a350b4b5788fa0a304f2695d9e8 ("[PATCH] ntp: use memset and offsetof init")
url: https://github.com/0day-ci/linux/commits/Yejune-Deng/ntp-use-memset-and-offsetof-init/20210120-110830
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git 3cabca87b329cbcbdf295be0094adbd72c7b1f67
in testcase: trinity
version: trinity-static-i386-x86_64-f93256fb_2019-08-28
with following parameters:
runtime: 300s
test-description: Trinity is a linux system call fuzz tester.
test-url: http://codemonkey.org.uk/projects/trinity/
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+-----------------------------------------------------------------------------------------+------------+------------+
| | 3cabca87b3 | a29bace5d3 |
+-----------------------------------------------------------------------------------------+------------+------------+
| boot_successes | 18 | 4 |
| boot_failures | 3 | 30 |
| invoked_oom-killer:gfp_mask=0x | 2 | 1 |
| Mem-Info | 2 | 1 |
| BUG:kernel_hang_in_boot_stage | 1 | 1 |
| BUG:Bad_rss-counter_state_mm:#type:MM_FILEPAGES_val | 0 | 5 |
| BUG:Bad_rss-counter_state_mm:#type:MM_ANONPAGES_val | 0 | 5 |
| BUG:Bad_rss-counter_state_mm:#type:MM_SHMEMPAGES_val | 0 | 5 |
| BUG:unable_to_handle_page_fault_for_address | 0 | 7 |
| Oops:#[##] | 0 | 19 |
| EIP:vt_console_print | 0 | 6 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 17 |
| EIP:native_machine_emergency_restart | 0 | 6 |
| Kernel_panic-not_syncing:Fatal_exception_in_interrupt | 0 | 9 |
| WARNING:at_kernel/events/core.c:#perf_event_groups_delete | 0 | 4 |
| EIP:perf_event_groups_delete | 0 | 4 |
| BUG:kernel_NULL_pointer_dereference,address | 0 | 12 |
| EIP:shmem_fault | 0 | 1 |
| EIP:__copy_user_ll | 0 | 1 |
| EIP:enqueue_task_fair | 0 | 2 |
| EIP:default_idle | 0 | 2 |
| EIP:strlen | 0 | 1 |
| EIP:filp_close | 0 | 2 |
| EIP:__splice_from_pipe | 0 | 1 |
| BUG:non-zero_pgtables_bytes_on_freeing_mm | 0 | 4 |
| EIP:unmap_page_range | 0 | 1 |
| PANIC:double_fault | 0 | 1 |
| double_fault:#[##] | 0 | 1 |
| EIP:no_context | 0 | 1 |
| EIP:queued_spin_lock_slowpath | 0 | 1 |
| EIP:list_lru_del | 0 | 2 |
| EIP:freeary | 0 | 1 |
| EIP:__perf_event_task_sched_in | 0 | 1 |
| Kernel_panic-not_syncing:stack-protector:Kernel_stack_is_corrupted_in:show_opcodes.cold | 0 | 1 |
| EIP:get_rr_interval_fair | 0 | 1 |
+-----------------------------------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
[ 67.177129] BUG: Bad rss-counter state mm:db2c2924 type:MM_FILEPAGES val:-2657
[ 67.178244] BUG: Bad rss-counter state mm:db2c2924 type:MM_ANONPAGES val:-814
[ 67.179219] BUG: Bad rss-counter state mm:db2c2924 type:MM_SHMEMPAGES val:-4591
[ 68.282064] audit: type=1326 audit(1611623366.489:46): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1192 comm="trinity-c5" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 69.314061] audit: type=1326 audit(1611623367.521:47): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1216 comm="trinity-c1" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 69.329549] audit: type=1326 audit(1611623367.537:48): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1083 comm="trinity-c6" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 69.352702] audit: type=1326 audit(1611623367.560:49): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1228 comm="trinity-c6" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 71.391986] audit: type=1326 audit(1611623369.598:50): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1241 comm="trinity-c6" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 72.127879] audit: type=1326 audit(1611623370.335:51): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1225 comm="trinity-c1" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 73.147217] [main] 31727 iterations. [F:23666 S:7901 HI:2077]
[ 73.147222]
[ 73.151518] audit: type=1326 audit(1611623371.358:52): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1258 comm="trinity-c0" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 73.205068] audit: type=1326 audit(1611623371.412:53): auid=4294967295 uid=65534 gid=65534 ses=4294967295 pid=1256 comm="trinity-c1" exe="/bin/trinity" sig=9 arch=40000003 syscall=19 compat=0 ip=0xb7f60549 code=0x0
[ 74.557386] trinity-c0 invoked oom-killer: gfp_mask=0x100cca(GFP_HIGHUSER_MOVABLE), order=0, oom_score_adj=500
[ 74.558865] CPU: 0 PID: 1269 Comm: trinity-c0 Not tainted 5.10.0-rc1-ga29bace5d3c0 #1
[ 74.560004] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 74.561175] Call Trace:
[ 74.561587] dump_stack (kbuild/src/consumer/lib/dump_stack.c:120)
[ 74.562084] dump_header (kbuild/src/consumer/mm/oom_kill.c:462)
[ 74.562616] oom_kill_process.cold (kbuild/src/consumer/mm/oom_kill.c:978)
[ 74.563241] ? oom_badness (kbuild/src/consumer/mm/oom_kill.c:514)
[ 74.563887] out_of_memory (kbuild/src/consumer/mm/oom_kill.c:1115 kbuild/src/consumer/mm/oom_kill.c:1047)
[ 74.564455] __alloc_pages_slowpath+0xaf4/0xba0
[ 74.565276] __alloc_pages_nodemask (kbuild/src/consumer/mm/page_alloc.c:4956)
[ 74.568673] shmem_getpage_gfp+0x13c/0x710
[ 74.569389] ? finish_task_switch (kbuild/src/consumer/include/linux/perf_event.h:1219 kbuild/src/consumer/kernel/sched/core.c:3611)
[ 74.570037] ? __schedule (kbuild/src/consumer/kernel/sched/core.c:3777 kbuild/src/consumer/kernel/sched/core.c:4523)
[ 74.570593] shmem_fallocate (kbuild/src/consumer/mm/shmem.c:2857)
[ 74.571194] ? do_setitimer (kbuild/src/consumer/include/linux/hrtimer.h:423 kbuild/src/consumer/kernel/time/itimer.c:237)
[ 74.571782] vfs_fallocate (kbuild/src/consumer/fs/open.c:309)
[ 74.572351] ? vfs_fallocate (kbuild/src/consumer/fs/open.c:309)
[ 74.574028] ksys_fallocate (kbuild/src/consumer/include/linux/file.h:45 kbuild/src/consumer/fs/open.c:333)
[ 74.574590] __ia32_sys_ia32_fallocate (kbuild/src/consumer/arch/x86/kernel/sys_ia32.c:119)
[ 74.575264] __do_fast_syscall_32 (kbuild/src/consumer/arch/x86/entry/common.c:78 kbuild/src/consumer/arch/x86/entry/common.c:137)
[ 74.575861] do_fast_syscall_32 (kbuild/src/consumer/arch/x86/entry/common.c:160)
[ 74.576500] do_SYSENTER_32 (kbuild/src/consumer/arch/x86/entry/common.c:204)
[ 74.577083] entry_SYSENTER_32 (kbuild/src/consumer/arch/x86/entry/entry_32.S:953)
[ 74.577694] EIP: 0xb7f60549
[ 74.578173] Code: 03 74 c0 01 10 05 03 74 b8 01 10 06 03 74 b4 01 10 07 03 74 b0 01 10 08 03 74 d8 01 00 00 00 00 00 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90 90 90 90 8d 76 00 58 b8 77 00 00 00 cd 80 90 8d 76
All code
========
0: 03 74 c0 01 add 0x1(%rax,%rax,8),%esi
4: 10 05 03 74 b8 01 adc %al,0x1b87403(%rip) # 0x1b8740d
a: 10 06 adc %al,(%rsi)
c: 03 74 b4 01 add 0x1(%rsp,%rsi,4),%esi
10: 10 07 adc %al,(%rdi)
12: 03 74 b0 01 add 0x1(%rax,%rsi,4),%esi
16: 10 08 adc %cl,(%rax)
18: 03 74 d8 01 add 0x1(%rax,%rbx,8),%esi
1c: 00 00 add %al,(%rax)
1e: 00 00 add %al,(%rax)
20: 00 51 52 add %dl,0x52(%rcx)
23: 55 push %rbp
24: 89 e5 mov %esp,%ebp
26: 0f 34 sysenter
28: cd 80 int $0x80
2a:* 5d pop %rbp <-- trapping instruction
2b: 5a pop %rdx
2c: 59 pop %rcx
2d: c3 retq
2e: 90 nop
2f: 90 nop
30: 90 nop
31: 90 nop
32: 8d 76 00 lea 0x0(%rsi),%esi
35: 58 pop %rax
36: b8 77 00 00 00 mov $0x77,%eax
3b: cd 80 int $0x80
3d: 90 nop
3e: 8d .byte 0x8d
3f: 76 .byte 0x76
Code starting with the faulting instruction
===========================================
0: 5d pop %rbp
1: 5a pop %rdx
2: 59 pop %rcx
3: c3 retq
4: 90 nop
5: 90 nop
6: 90 nop
7: 90 nop
8: 8d 76 00 lea 0x0(%rsi),%esi
b: 58 pop %rax
c: b8 77 00 00 00 mov $0x77,%eax
11: cd 80 int $0x80
13: 90 nop
14: 8d .byte 0x8d
15: 76 .byte 0x76
[ 74.580624] EAX: ffffffda EBX: 00000117 ECX: 00000000 EDX: 00001000
[ 74.581445] ESI: 00000004 EDI: 00020000 EBP: 00000064 ESP: bf91ce9c
[ 74.582280] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000292
[ 74.583236] Mem-Info:
[ 74.585128] active_anon:5960 inactive_anon:722051 isolated_anon:0
[ 74.585128] active_file:0 inactive_file:0 isolated_file:0
[ 74.585128] unevictable:36763 dirty:0 writeback:0
[ 74.585128] slab_reclaimable:5105 slab_unreclaimable:2319
[ 74.585128] mapped:7877 shmem:711742 pagetables:314 bounce:0
[ 74.585128] free:1409 free_pcp:217 free_cma:0
[ 74.594070] Node 0 active_anon:23840kB inactive_anon:2888204kB active_file:0kB inactive_file:0kB unevictable:147052kB isolated(anon):0kB isolated(file):0kB mapped:31508kB dirty:0kB writeback:0kB shmem:2846968kB writeback_tmp:0kB kernel_stack:688kB all_unreclaimable? yes
[ 74.600900] DMA free:1652kB min:96kB low:120kB high:144kB reserved_highatomic:0KB active_anon:0kB inactive_anon:14236kB active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB present:15992kB managed:15916kB mlocked:0kB pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
[ 74.600902] lowmem_reserve[]: 0 392 2975 392
[ 74.609397] Normal free:3260kB min:8068kB low:8688kB high:9308kB reserved_highatomic:0KB active_anon:0kB inactive_anon:392740kB active_file:0kB inactive_file:0kB unevictable:8760kB writepending:0kB present:483320kB managed:437924kB mlocked:0kB pagetables:0kB bounce:0kB free_pcp:620kB local_pcp:620kB free_cma:0kB
[ 74.609401] lowmem_reserve[]: 0 0 20671 0
[ 74.618257] HighMem free:724kB min:4608kB low:8700kB high:12792kB reserved_highatomic:0KB active_anon:23840kB inactive_anon:2481124kB active_file:0kB inactive_file:0kB unevictable:138292kB writepending:0kB present:2645896kB managed:2645896kB mlocked:36kB pagetables:1256kB bounce:0kB free_pcp:248kB local_pcp:248kB free_cma:0kB
[ 74.618258] lowmem_reserve[]: 0 0 0 0
[ 74.627934] DMA: 1*4kB (U) 2*8kB (UM) 0*16kB 1*32kB (U) 3*64kB (UM) 1*128kB (U) 1*256kB (U) 0*512kB 1*1024kB (M) 0*2048kB 0*4096kB = 1652kB
[ 74.631546] Normal: 0*4kB 110*8kB (UE) 71*16kB (UE) 33*32kB (UE) 1*64kB (E) 0*128kB 0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 3136kB
[ 74.635123] HighMem: 49*4kB (UM) 18*8kB (UM) 2*16kB (UM) 1*32kB (U) 1*64kB (M) 0*128kB 1*256kB (M) 0*512kB 0*1024kB 0*2048kB 0*4096kB = 724kB
[ 74.636824] Node 0 hugepages_total=0 hugepages_free=0 hugepages_surp=0 hugepages_size=4096kB
[ 74.641687] 748496 total pagecache pages
[ 74.642302] 0 pages in swap cache
[ 74.642814] Swap cache stats: add 0, delete 0, find 0/0
[ 74.644272] Free swap = 0kB
[ 74.645216] Total swap = 0kB
[ 74.646094] 786302 pages RAM
[ 74.646620] 661474 pages HighMem/MovableOnly
[ 74.648037] 11368 pages reserved
[ 74.648612] Tasks state (memory values in pages):
[ 74.649995] [ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj name
[ 74.652122] [ 71] 0 71 869 628 16384 0 0 plymouthd
[ 74.653402] [ 73] 0 73 856 442 16384 0 0 lkp-bootstrap
[ 74.656217] [ 75] 0 75 1362 887 20480 0 -1000 lkp-setup-rootf
[ 74.657495] [ 95] 0 95 969 323 16384 0 -1000 tail
[ 74.660090] [ 96] 0 96 9170 8337 49152 0 -1000 sed
[ 74.661260] [ 97] 0 97 968 318 16384 0 -1000 tail
[ 74.663876] [ 98] 0 98 6535 5658 45056 0 -1000 sed
[ 74.665035] [ 169] 0 169 705 456 16384 0 0 upstart-udev-br
[ 74.667784] [ 176] 0 176 775 619 16384 0 -1000 udevd
[ 74.668966] [ 180] 0 180 1416 917 20480 0 -1000 run-lkp
[ 74.671455] [ 254] 0 254 742 460 16384 0 -1000 udevd
[ 74.673566] [ 268] 0 268 742 459 16384 0 -1000 udevd
[ 74.674747] [ 356] 0 356 702 36 16384 0 0 upstart-socket-
[ 74.677586] [ 375] 0 375 882 740 16384 0 0 rc
[ 74.678680] [ 376] 0 376 645 472 16384 0 0 getty
[ 74.681308] [ 378] 0 378 645 481 16384 0 0 getty
[ 74.682478] [ 379] 0 379 645 485 16384 0 0 getty
[ 74.684994] [ 382] 0 382 645 470 16384 0 0 getty
[ 74.686182] [ 383] 0 383 645 443 16384 0 0 getty
[ 74.688870] [ 394] 0 394 870 747 16384 0 0 S99rc.local
[ 74.690122] [ 400] 0 400 836 665 16384 0 0 rc.local
[ 74.692726] [ 401] 0 401 837 680 16384 0 0 lkp-bootstrap
[ 74.693980] [ 404] 0 404 537 135 16384 0 0 sleep
[ 74.696210] [ 407] 0 407 1319 830 20480 0 1000 trinity-300s-qu
[ 74.698619] [ 411] 0 411 970 291 12288 0 0 cat
[ 74.699764] [ 413] 0 413 1001 369 16384 0 0 vmstat
[ 74.702157] [ 415] 0 415 1280 789 20480 0 0 meminfo
[ 74.704115] [ 431] 0 431 244 11 12288 0 0 wakeup
[ 74.705232] [ 433] 0 433 244 12 16384 0 0 wakeup
[ 74.707438] [ 442] 0 442 966 287 20480 0 0 cat
[ 74.709516] [ 446] 0 446 638 425 12288 0 0 gzip-meminfo
[ 74.710764] [ 457] 0 457 1280 774 20480 0 0 trinity
[ 74.713267] [ 460] 0 460 961 368 16384 0 0 tee
To reproduce:
# build kernel
cd linux
cp config-5.10.0-rc1-ga29bace5d3c0 .config
make HOSTCC=gcc-9 CC=gcc-9 ARCH=i386 olddefconfig prepare modules_prepare bzImage
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks,
Oliver Sang
On Wed, Jan 20 2021 at 10:51, Yejune Deng wrote:
> In pps_fill_timex(), use memset and offsetof instead of '= 0'.
>
> Signed-off-by: Yejune Deng <[email protected]>
> ---
> kernel/time/ntp.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> index 87389b9e21ab..3416c0381104 100644
> --- a/kernel/time/ntp.c
> +++ b/kernel/time/ntp.c
> @@ -225,14 +225,11 @@ static inline int is_error_status(int status)
> static inline void pps_fill_timex(struct __kernel_timex *txc)
> {
> /* PPS is not implemented, so these are zero */
> - txc->ppsfreq = 0;
> - txc->jitter = 0;
> - txc->shift = 0;
> - txc->stabil = 0;
> - txc->jitcnt = 0;
> - txc->calcnt = 0;
> - txc->errcnt = 0;
> - txc->stbcnt = 0;
> + int offset, len;
> +
> + offset = offsetof(struct __kernel_timex, ppsfreq);
> + len = offsetof(struct __kernel_timex, tai) - offset;
> + memset(txc + offset, 0, len);
That zeros bytes at a memory location which is
(offset) * sizeof(struct __kernel_timex)
bytes away from txc. How did this every boot?
And no, even if you fix that pointer math problem then this kind of
calculation from the middle of a struct is error prone.
Thanks,
tglx
From: Thomas Gleixner
> Sent: 05 February 2021 17:34
>
> On Wed, Jan 20 2021 at 10:51, Yejune Deng wrote:
> > In pps_fill_timex(), use memset and offsetof instead of '= 0'.
> >
> > Signed-off-by: Yejune Deng <[email protected]>
> > ---
> > kernel/time/ntp.c | 13 +++++--------
> > 1 file changed, 5 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c
> > index 87389b9e21ab..3416c0381104 100644
> > --- a/kernel/time/ntp.c
> > +++ b/kernel/time/ntp.c
> > @@ -225,14 +225,11 @@ static inline int is_error_status(int status)
> > static inline void pps_fill_timex(struct __kernel_timex *txc)
> > {
> > /* PPS is not implemented, so these are zero */
> > - txc->ppsfreq = 0;
> > - txc->jitter = 0;
> > - txc->shift = 0;
> > - txc->stabil = 0;
> > - txc->jitcnt = 0;
> > - txc->calcnt = 0;
> > - txc->errcnt = 0;
> > - txc->stbcnt = 0;
> > + int offset, len;
> > +
> > + offset = offsetof(struct __kernel_timex, ppsfreq);
> > + len = offsetof(struct __kernel_timex, tai) - offset;
> > + memset(txc + offset, 0, len);
>
> That zeros bytes at a memory location which is
>
> (offset) * sizeof(struct __kernel_timex)
>
> bytes away from txc. How did this every boot?
>
> And no, even if you fix that pointer math problem then this kind of
> calculation from the middle of a struct is error prone.
It is also, at best, a code size optimisation.
If memset() is actually called (not inlined) then you get a whole
lot of tests against the size and alignment before any writes
of zero happen - which will be the same ones as in the inline code.
It can be worth using memcpy to copy part of a structure
but usually for one with lots of small fields (especially bitfields).
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)