Re: KASAN: use-after-free Read in handle_userfault (2)
On Wed, Dec 12, 2018 at 10:58 AM Dmitry Vyukov wrote: > > On Wed, Dec 12, 2018 at 10:45 AM syzbot > wrote: > > > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:14cf8c1d5b90 Add linux-next specific files for 20181210 > > git tree: linux-next > > console output: https://syzkaller.appspot.com/x/log.txt?x=133296db40 > > kernel config: https://syzkaller.appspot.com/x/.config?x=c9133d0a4284c012 > > dashboard link: https://syzkaller.appspot.com/bug?extid=cbc64b24b2b2d54c07a9 > > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > > This is a corrupted/intermixed kernel output, the bug actually > happened somewhere in net/core/neighbour.c. > syzkaller has a bunch of code to try to deal to corrupted kernel > output, but it's not always possible as a parallel thread printing > something can inject an unrelated frame just in the right place, and > then it's indistinguishable from a crash happened there. > > The question is: what's the significance of that > "FAULT_FLAG_ALLOW_RETRY missing 70"? > Is it something to fix in kernel? Can the check be removed? Should it > be moved somewhere earlier when flags are passed to kernel and cause > EINVAL? Can it be at least converted to a single-line pr_warn? > +Sasha go as far as suggesting that any "Call Trace:" in kernel output > means a kernel bugs. This is one of few places in kernel that dumps > stacks left and right without a kernel bug (?). up anybody maintaining uffd? > > Unfortunately, I don't have any reproducer for this crash yet. > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+cbc64b24b2b2d54c0...@syzkaller.appspotmail.com > > > > RDX: 03ff RSI: 20012fe0 RDI: 7f5dbe489850 > > RBP: 0072bf00 R08: 03ff R09: > > R10: R11: 0246 R12: 7f5dbe48a6d4 > > R13: 004c578a R14: 004d9d90 R15: > > == > > BUG: KASAN: use-after-free in __list_del_entry_valid+0xf1/0x100 > > lib/list_debug.c:51 > > CPU: 1 PID: 20306 Comm: syz-executor2 Not tainted 4.20.0-rc6-next-20181210+ > > #164 > > Read of size 8 at addr 8881c5e72bb0 by task kworker/0:1/12 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > > > Call Trace: > > __dump_stack lib/dump_stack.c:77 [inline] > > dump_stack+0x244/0x39d lib/dump_stack.c:113 > > handle_userfault.cold.30+0x47/0x62 fs/userfaultfd.c:431 > > do_anonymous_page mm/memory.c:2938 [inline] > > handle_pte_fault mm/memory.c:3780 [inline] > > __handle_mm_fault+0x4d26/0x5b70 mm/memory.c:3906 > > handle_mm_fault+0x54f/0xc70 mm/memory.c:3943 > > do_user_addr_fault arch/x86/mm/fault.c:1475 [inline] > > __do_page_fault+0x5f6/0xd70 arch/x86/mm/fault.c:1541 > > do_page_fault+0xf2/0x7e0 arch/x86/mm/fault.c:1572 > > page_fault+0x1e/0x30 arch/x86/entry/entry_64.S:1143 > > RIP: 0033:0x4510a0 > > Code: 0f 84 c4 0f 00 00 48 89 f1 48 89 f8 48 83 e1 3f 48 83 f9 20 0f 86 7b > > 02 00 00 48 83 e6 f0 48 83 e1 0f 66 0f ef c0 66 0f ef c9 <66> 0f 74 0e 66 > > 0f d7 d1 48 d3 ea 49 c7 c2 11 00 00 00 49 29 ca 4d > > RSP: 002b:7fab1fbba7a8 EFLAGS: 00010202 > > RAX: 7fab1fbba850 RBX: 0003 RCX: 000e > > RDX: 03ff RSI: 20012fe0 RDI: 7fab1fbba850 > > RBP: 0072bf00 R08: 03ff R09: > > R10: R11: 0246 R12: 7fab1fbbb6d4 > > R13: 004c578a R14: 004d9d90 R15: > > CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 4.20.0-rc6-next-20181210+ #164 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > Workqueue: events_power_efficient neigh_periodic_work > > Call Trace: > > __dump_stack lib/dump_stack.c:77 [inline] > > dump_stack+0x244/0x39d lib/dump_stack.c:113 > > print_address_description.cold.4+0x9/0x1ff mm/kasan/report.c:187 > > kasan_report.cold.5+0x1b/0x39 mm/kasan/report.c:317 > > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135 > > __list_del_entry_valid+0xf1/0x100 lib/list_debug.c:51 > > __list_del_entry include/linux/list.h:117 [inline] > > list_del_init include/linux/list.h:159 [inline] > > neigh_mark_dead+0x13b/0x410 net/core/neighbour.c:125 > > neigh_periodic_work+0x89a/0xc30 net/core/neighbour.c:905 > > process_one_work+0xc90/0x1c40 kernel/workqueue.c:2153 > > worker_thread+0x17f/0x1390 kernel/workqueue.c:2296 > > kthread+0x35a/0x440 kernel/kthread.c:246 > > ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352 > > > > Allocated by task 8166: > > save_stack+0x43/0xd0 mm/kasan/common.c:73 > > set_track mm/kasan/common.c:85 [inline] > > kasan_kmalloc+0xcb/0xd0 mm/kasan/common.c:482 > > __do_kmalloc_node mm/slab.c:3671 [inline] > > __kmalloc_node_track_caller+0x4d/0x70
Re: [PATCH] netfilter: account ebt_table_info to kmemcg
On Sat 29-12-18 11:34:29, Shakeel Butt wrote: > On Sat, Dec 29, 2018 at 2:06 AM Michal Hocko wrote: > > > > On Sat 29-12-18 10:52:15, Florian Westphal wrote: > > > Michal Hocko wrote: > > > > On Fri 28-12-18 17:55:24, Shakeel Butt wrote: > > > > > The [ip,ip6,arp]_tables use x_tables_info internally and the > > > > > underlying > > > > > memory is already accounted to kmemcg. Do the same for ebtables. The > > > > > syzbot, by using setsockopt(EBT_SO_SET_ENTRIES), was able to OOM the > > > > > whole system from a restricted memcg, a potential DoS. > > > > > > > > What is the lifetime of these objects? Are they bound to any process? > > > > > > No, they are not. > > > They are free'd only when userspace requests it or the netns is > > > destroyed. > > > > Then this is problematic, because the oom killer is not able to > > guarantee the hard limit and so the excessive memory consumption cannot > > be really contained. As a result the memcg will be basically useless > > until somebody tears down the charged objects by other means. The memcg > > oom killer will surely kill all the existing tasks in the cgroup and > > this could somehow reduce the problem. Maybe this is sufficient for > > some usecases but that should be properly analyzed and described in the > > changelog. > > > > Can you explain why you think the memcg hard limit will not be > enforced? From what I understand, the memcg oom-killer will kill the > allocating processes as you have mentioned. We do force charging for > very limited conditions but here the memcg oom-killer will take care > of I was talking about the force charge part. Depending on a specific allocation and its life time this can gradually get us over hard limit without any bound theoretically. > Anyways, the kernel is already charging the memory for > [ip,ip6,arp]_tables and this patch adds the charging for ebtables. > Without this patch, as Kirill has described and shown by syzbot, a low > priority memcg can force system OOM. I am not opposing the patch per-se. I would just like the changelog to be more descriptive about the life time and consequences. -- Michal Hocko SUSE Labs
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Sun, Dec 30, 2018 at 08:08:46AM +0100, Willy Tarreau wrote: > Definitely! Same, I won't emit a patch just for this, Paul already queued it. Yeah, not that big a deal :) Reviewed-by: Joey Pabalinas -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH v8 13/25] m68k: Dispatch nvram_ops calls to Atari or Mac functions
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
> On Wed, Dec 26, 2018 at 1:43 AM Finn Thain wrote:
>
> > +
> > +static ssize_t m68k_nvram_get_size(void)
> > +{
> > + if (MACH_IS_ATARI)
> > + return atari_nvram_get_size();
> > + else if (MACH_IS_MAC)
> > + return mac_pram_get_size();
> > + return -ENODEV;
> > +}
> > +
> > +/* Atari device drivers call .read (to get checksum validation) whereas
> > + * Mac and PowerMac device drivers just use .read_byte.
> > + */
> > +const struct nvram_ops arch_nvram_ops = {
> > +#ifdef CONFIG_MAC
> > + .read_byte = m68k_nvram_read_byte,
> > + .write_byte = m68k_nvram_write_byte,
> > +#endif
> > +#ifdef CONFIG_ATARI
> > + .read = m68k_nvram_read,
> > + .write = m68k_nvram_write,
> > + .set_checksum = m68k_nvram_set_checksum,
> > + .initialize = m68k_nvram_initialize,
> > +#endif
> > + .get_size = m68k_nvram_get_size,
> > +};
> > +EXPORT_SYMBOL(arch_nvram_ops);
>
> Since the operations are almost entirely distinct, why not have two
> separate 'nvram_ops' instances here that each refer to just
> the set they actually need?
>
The reason for that is that I am alergic to code duplication. But I'll
change it if you think it matters. BTW, this patch has already been acked
by Geert.
> I was actually expecting one more patch here that would make the
> arch_nvram_ops a pointer to one of multiple structures, which would
> be easier to do with multiple copies, but I suppose there is no need
> for that here (there might be on ppc, I have to look again).
>
Yes, I considered that too. I picked the variation that makes everything
const.
--
>Arnd
>
Re: [PATCH v8 18/25] powerpc: Implement nvram sync ioctl
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
> > --- a/drivers/char/nvram.c
> > +++ b/drivers/char/nvram.c
> > @@ -48,6 +48,10 @@
> > #include
> > #include
> >
> > +#ifdef CONFIG_PPC
> > +#include
> > +#include
> > +#endif
> >
> > static DEFINE_MUTEX(nvram_mutex);
> > static DEFINE_SPINLOCK(nvram_state_lock);
> > @@ -331,6 +335,37 @@ static long nvram_misc_ioctl(struct file *file,
> > unsigned int cmd,
> > long ret = -ENOTTY;
> >
> > switch (cmd) {
> > +#ifdef CONFIG_PPC
> > + case OBSOLETE_PMAC_NVRAM_GET_OFFSET:
> > + pr_warn("nvram: Using obsolete PMAC_NVRAM_GET_OFFSET
> > ioctl\n");
> > + /* fall through */
> > + case IOC_NVRAM_GET_OFFSET:
> > + ret = -EINVAL;
> > +#ifdef CONFIG_PPC_PMAC
>
> I think it would make be nicer here to keep the ppc bits in arch/ppc,
> and instead add a .ioctl() callback to nvram_ops.
>
The problem with having an nvram_ops.ioctl() method is the code in the
!PPC branch. That code would get duplicated because it's needed by both
X86 and M68K, to implement the checksum ioctls.
> > @@ -369,12 +405,14 @@ static int nvram_misc_open(struct inode *inode,
> > struct file *file)
> > return -EBUSY;
> > }
> >
> > +#ifndef CONFIG_PPC
> > /* Prevent multiple writers if the set_checksum ioctl is
> > implemented. */
> > if ((arch_nvram_ops.set_checksum != NULL) &&
> > (file->f_mode & FMODE_WRITE) && (nvram_open_mode &
> > NVRAM_WRITE)) {
> > spin_unlock(_state_lock);
> > return -EBUSY;
> > }
> > +#endif
> >
> > diff --git a/include/linux/nvram.h b/include/linux/nvram.h
> > index b7bfaec60a43..24a57675dba1 100644
> > --- a/include/linux/nvram.h
> > +++ b/include/linux/nvram.h
> > @@ -18,8 +18,12 @@ struct nvram_ops {
> > unsigned char (*read_byte)(int);
> > void(*write_byte)(unsigned char, int);
> > ssize_t (*get_size)(void);
> > +#ifdef CONFIG_PPC
> > + long(*sync)(void);
> > +#else
> > long(*set_checksum)(void);
> > long(*initialize)(void);
> > +#endif
> > };
>
> Maybe just leave all entries visible here, and avoid the above #ifdef checks.
>
The #ifdef isn't there just to save a few bytes, though it does do that.
It's really meant to cause a build failure when I mess up somewhere. But
I'm happy to change it if you can see a reason to do so (?)
--
>Arnd
>
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Sat, Dec 29, 2018 at 12:33:24PM -1000, Joey Pabalinas wrote: > On Sat, Dec 29, 2018 at 07:02:18PM +0100, Willy Tarreau wrote: > > + * - the lower level is the arch-specific syscall() definition, > > consisting in > > + * assembly code in compound expressions. These are called > > my_syscall0() to > > + * my_syscall6() depending on the number of arguments. The MIPS > > + * implementation is limited to 5 arguments. All input arguments are > > cast > > + * to a long stored in a register. These expressions always return the > > + * syscall's return value as a signed long value which is often either > > a > > + * pointer or the negated errno value. > > + * > > + * - the second level is mostly architecture-independent. It is made of > > + * static functions called sys_() which rely on my_syscallN() > > + * depending on the syscall definition. These functions are responsible > > + * for exposing the appropriate types for the syscall arguments (int, > > + * pointers, etc) and for setting the appropriate return type (often > > int). > > + * A few of them are architecture-specific because the syscalls are > > not all > > + * mapped exactly the same among architectures. For example, some > > archs do > > + * not implement select() and need pselect6() instead, so the > > sys_select() > > + * function will have to abstract this. > > + * > > + * - the third level is the libc call definition. It exposes the lower > > raw > > + * sys_() calls in a way that looks like what a libc usually > > does, > > + * takes care of specific input values, and of setting errno upon > > error. > > + * There can be minor variations compared to standard libc calls. For > > + * example the open() call always takes 3 args here. > > Shouldn't these sentences begin with a capitalized "The" for > consistency? Not sure since they're just list items. But probably as such they should end with a semi-colon and not a dot. Anyway, this is minor and likely for a later update to the file. > > /* some archs (at least aarch64) don't expose the regular syscalls anymore > > by > > * default, either because they have an "_at" replacement, or because > > there are > > * more modern alternatives. For now we'd rather still use them. > > Also here. Shouldn't this begin with a capitalized "Some"? Definitely! Same, I won't emit a patch just for this, Paul already queued it. Thanks! Willy
WARNING: lock held when returning to user space in set_property_atomic
Hello,
syzbot found the following crash on:
HEAD commit:903b77c63167 Merge tag 'linux-kselftest-4.21-rc1' of git:/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12d0f55340
kernel config: https://syzkaller.appspot.com/x/.config?x=53a2f2aa0b1f7606
dashboard link: https://syzkaller.appspot.com/bug?extid=6ea337c427f5083ebdf2
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=120d906f40
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1024673b40
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6ea337c427f5083eb...@syzkaller.appspotmail.com
RBP: 7ffe369ca7a0 R08: 0001 R09: 004009ce
R10: R11: 0246 R12: 0005
R13: R14: R15:
WARNING: lock held when returning to user space!
4.20.0+ #174 Not tainted
syz-executor556/8153 is leaving the kernel with locks still held!
1 lock held by syz-executor556/8153:
#0: 5100c85c (crtc_ww_class_acquire){+.+.}, at:
set_property_atomic+0xb3/0x330 drivers/gpu/drm/drm_mode_object.c:462
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkal...@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
BUG: unable to handle kernel paging request in sctp_v6_get_dst
Hello, syzbot found the following crash on: HEAD commit:90cadbbf341d Merge git://git.kernel.org/pub/scm/linux/kern.. git tree: net-next console output: https://syzkaller.appspot.com/x/log.txt?x=1081829b40 kernel config: https://syzkaller.appspot.com/x/.config?x=9d41c8529d7e7362 dashboard link: https://syzkaller.appspot.com/bug?extid=ae70faffd84f05295f27 compiler: gcc (GCC) 8.0.1 20180413 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+ae70faffd84f05295...@syzkaller.appspotmail.com dst_release: dst:6e60b654 refcnt:-1 dst_release: dst:6e60b654 refcnt:-1 BUG: unable to handle kernel paging request at 88f1 PGD 966d067 P4D 966d067 PUD 966f067 PMD 0 Oops: [#1] PREEMPT SMP KASAN CPU: 0 PID: 26041 Comm: syz-executor2 Not tainted 4.20.0-rc7+ #360 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:__read_once_size include/linux/compiler.h:191 [inline] RIP: 0010:rt6_get_cookie include/net/ip6_fib.h:260 [inline] RIP: 0010:sctp_v6_get_dst+0x9d1/0x22d0 net/sctp/ipv6.c:379 Code: c1 ea 03 c6 04 02 00 48 8d 79 70 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 7e 18 00 00 48 8b 85 a8 fc ff ff 48 89 da 48 c1 ea 03 <4c> 8b 60 70 48 b8 00 00 00 00 00 fc ff df 80 3c 02 00 0f 85 6f 18 RSP: 0018:8881dac062b0 EFLAGS: 00010a02 RAX: 8881 RBX: 8881dac06510 RCX: 8881 RDX: 11103b580ca2 RSI: 874b895b RDI: 88f1 RBP: 8881dac06660 R08: 888194e86600 R09: ed103b585b77 R10: ed103b585b77 R11: 8881dac2dbbb R12: 8881b12a1d80 R13: dc00 R14: ed103b580c7a R15: 8881dac065d0 FS: 7f2d362c2700() GS:8881dac0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 88f1 CR3: 0001d1f76000 CR4: 001406f0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: sctp_transport_route+0x132/0x360 net/sctp/transport.c:311 sctp_assoc_add_peer+0x5fd/0x10d0 net/sctp/associola.c:678 sctp_process_param net/sctp/sm_make_chunk.c:2543 [inline] sctp_process_init+0xfba/0x29c0 net/sctp/sm_make_chunk.c:2356 sctp_sf_do_5_1B_init+0x8e3/0xf70 net/sctp/sm_statefuns.c:416 sctp: [Deprecated]: syz-executor4 (pid 26043) Use of int in max_burst socket option deprecated. Use struct sctp_assoc_value instead sctp_do_sm+0x1c1/0x7190 net/sctp/sm_sideeffect.c:1188 kobject: 'loop4' (7c488f81): kobject_uevent_env kobject: 'loop4' (7c488f81): fill_kobj_path: path = '/devices/virtual/block/loop4' sctp_endpoint_bh_rcv+0x465/0x960 net/sctp/endpointola.c:456 sctp_inq_push+0x280/0x370 net/sctp/inqueue.c:95 kobject: 'loop4' (7c488f81): kobject_uevent_env sctp_rcv+0x2e48/0x4150 net/sctp/input.c:271 kobject: 'loop4' (7c488f81): fill_kobj_path: path = '/devices/virtual/block/loop4' sctp6_rcv+0x15/0x30 net/sctp/ipv6.c:1065 ip6_protocol_deliver_rcu+0x372/0x1940 net/ipv6/ip6_input.c:394 ip6_input_finish+0x84/0x170 net/ipv6/ip6_input.c:434 NF_HOOK include/linux/netfilter.h:289 [inline] ip6_input+0xe9/0x600 net/ipv6/ip6_input.c:443 dst_input include/net/dst.h:450 [inline] ip6_rcv_finish+0x17a/0x330 net/ipv6/ip6_input.c:76 NF_HOOK include/linux/netfilter.h:289 [inline] ipv6_rcv+0x115/0x640 net/ipv6/ip6_input.c:272 __netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4973 __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5083 process_backlog+0x217/0x760 net/core/dev.c:5923 napi_poll net/core/dev.c:6346 [inline] net_rx_action+0x7c5/0x1950 net/core/dev.c:6412 __do_softirq+0x30c/0xb2e kernel/softirq.c:292 do_softirq_own_stack+0x2a/0x40 arch/x86/entry/entry_64.S:1027 do_softirq.part.13+0x126/0x160 kernel/softirq.c:337 do_softirq kernel/softirq.c:329 [inline] __local_bh_enable_ip+0x21d/0x260 kernel/softirq.c:189 local_bh_enable include/linux/bottom_half.h:32 [inline] rcu_read_unlock_bh include/linux/rcupdate.h:696 [inline] ip6_finish_output2+0xcef/0x2930 net/ipv6/ip6_output.c:121 ip6_finish_output+0x583/0xc50 net/ipv6/ip6_output.c:154 NF_HOOK_COND include/linux/netfilter.h:278 [inline] ip6_output+0x232/0x9d0 net/ipv6/ip6_output.c:171 dst_output include/net/dst.h:444 [inline] NF_HOOK include/linux/netfilter.h:289 [inline] ip6_xmit+0xf0d/0x24c0 net/ipv6/ip6_output.c:275 sctp_v6_xmit+0x385/0x760 net/sctp/ipv6.c:234 sctp_packet_transmit+0x1f05/0x3cd0 net/sctp/output.c:641 sctp_packet_singleton net/sctp/outqueue.c:787 [inline] sctp_outq_flush_ctrl.constprop.11+0x7a9/0xe50 net/sctp/outqueue.c:918 sctp_outq_flush+0x310/0x34f0 net/sctp/outqueue.c:1200 sctp_outq_uncork+0x6a/0x80 net/sctp/outqueue.c:772 sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1820 [inline] sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline] sctp_do_sm+0x5ff/0x7190
Re: [PATCH v1 0/2] Virtio: fix some vq allocation issues
On Sat, 29 Dec 2018 02:45:49 + "Wang, Wei W" wrote: > On Friday, December 28, 2018 3:57 PM, Christian Borntraeger wrote: > > On 28.12.2018 03:26, Wei Wang wrote: > > > Some vqs don't need to be allocated when the related feature bits are > > > disabled. Callers notice the vq allocation layer by setting the > > > related names[i] to be NULL. > > > > > > This patch series fixes the find_vqs implementations to handle this case. > > > > So the random crashes during boot are gone. > > What still does not work is actually using the balloon. > > > > So in the qemu monitor using lets say "balloon 1000" will hang the guest. > > Seems to be a deadlock in the virtio-ccw code. We seem to call the config > > code in the interrupt handler. > > Yes. It reads a config register from the interrupt handler. Do you know why > ccw doesn't support it and has some internal lock that caused the deadlock > issue? > > Best, > Wei I guess you are the first one trying to read virtio config from within interrupt context. AFAICT this never worked. About what happens. The apidoc of ccw_device_start() says it needs to be called with the ccw device lock held, so ccw_io_helper() tries to take it (since forever I guess). OTOH do_cio_interrupt() takes the subchannel lock and io_subchannel_initialize_dev() makes the ccw device lock be the subchannel lock. That means when one tries to get virtio config form within a cio interrupt context we deadlock, because we try to take a lock we already have. That said, I don't think this limitation is by design (i.e. intended). Maybe Connie can help us with that question. AFAIK we have nothing documented regarding this (neither that can nor can't). Obviously, there are multiple ways around this problem, and at the moment I can't tell which would be my preferred one. Regards, Halil
sound/pci//hda/patch_ca0132.c:8416:3: error: implicit declaration of function 'pci_iounmap'; did you mean 'pcim_iounmap'?
Hi Takashi,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: 195303136f192d37b89e20a8d1d2670d0d825266
commit: d99501b8575dc1248bacf1b58d2241cb4b265d49 ALSA: hda/ca0132 - Call
pci_iounmap() instead of iounmap()
date: 7 weeks ago
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout d99501b8575dc1248bacf1b58d2241cb4b265d49
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=sh
All errors (new ones prefixed by >>):
sound/pci//hda/patch_ca0132.c: In function 'ca0132_free':
>> sound/pci//hda/patch_ca0132.c:8416:3: error: implicit declaration of
>> function 'pci_iounmap'; did you mean 'pcim_iounmap'?
>> [-Werror=implicit-function-declaration]
pci_iounmap(codec->bus->pci, spec->mem_base);
^~~
pcim_iounmap
sound/pci//hda/patch_ca0132.c: In function 'patch_ca0132':
sound/pci//hda/patch_ca0132.c:8799:20: error: implicit declaration of
function 'pci_iomap'; did you mean 'pcim_iomap'?
[-Werror=implicit-function-declaration]
spec->mem_base = pci_iomap(codec->bus->pci, 2, 0xC20);
^
pcim_iomap
sound/pci//hda/patch_ca0132.c:8799:18: warning: assignment makes pointer
from integer without a cast [-Wint-conversion]
spec->mem_base = pci_iomap(codec->bus->pci, 2, 0xC20);
^
cc1: some warnings being treated as errors
vim +8416 sound/pci//hda/patch_ca0132.c
8386
8387 static void ca0132_free(struct hda_codec *codec)
8388 {
8389 struct ca0132_spec *spec = codec->spec;
8390
8391 cancel_delayed_work_sync(>unsol_hp_work);
8392 snd_hda_power_up(codec);
8393 switch (spec->quirk) {
8394 case QUIRK_SBZ:
8395 sbz_exit_chip(codec);
8396 break;
8397 case QUIRK_ZXR:
8398 zxr_exit_chip(codec);
8399 break;
8400 case QUIRK_R3D:
8401 r3d_exit_chip(codec);
8402 break;
8403 case QUIRK_AE5:
8404 ae5_exit_chip(codec);
8405 break;
8406 case QUIRK_R3DI:
8407 r3di_gpio_shutdown(codec);
8408 break;
8409 }
8410
8411 snd_hda_sequence_write(codec, spec->base_exit_verbs);
8412 ca0132_exit_chip(codec);
8413
8414 snd_hda_power_down(codec);
8415 if (spec->mem_base)
> 8416 pci_iounmap(codec->bus->pci, spec->mem_base);
8417 kfree(spec->spec_init_verbs);
8418 kfree(codec->spec);
8419 }
8420
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
device_sysfs.c:undefined reference to `utf16s_to_utf8s'
Hi Sinan, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 195303136f192d37b89e20a8d1d2670d0d825266 commit: 5d32a66541c4683456507481a0944ed2985e75c7 PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set date: 10 days ago config: i386-alldefconfig (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: git checkout 5d32a66541c4683456507481a0944ed2985e75c7 # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/acpi/device_sysfs.o: In function `description_show': >> device_sysfs.c:(.text+0x44b): undefined reference to `utf16s_to_utf8s' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[v4 PATCH 1/2] mm: swap: check if swap backing device is congested or not
Swap readahead would read in a few pages regardless if the underlying
device is busy or not. It may incur long waiting time if the device is
congested, and it may also exacerbate the congestion.
Use inode_read_congested() to check if the underlying device is busy or
not like what file page readahead does. Get inode from swap_info_struct.
Although we can add inode information in swap_address_space
(address_space->host), it may lead some unexpected side effect, i.e.
it may break mapping_cap_account_dirty(). Using inode from
swap_info_struct seems simple and good enough.
Just does the check in vma_cluster_readahead() since
swap_vma_readahead() is just used for non-rotational device which
much less likely has congestion than traditional HDD.
Although swap slots may be consecutive on swap partition, it still may be
fragmented on swap file. This check would help to reduce excessive stall
for such case.
The test on my virtual machine with congested HDD shows long tail
latency is reduced significantly.
Without the patch
page_fault1_thr-1490 [023] 129.311706: funcgraph_entry: #57377.796 us
| do_swap_page();
page_fault1_thr-1490 [023] 129.369103: funcgraph_entry:5.642us |
do_swap_page();
page_fault1_thr-1490 [023] 129.369119: funcgraph_entry: #1289.592 us |
do_swap_page();
page_fault1_thr-1490 [023] 129.370411: funcgraph_entry:4.957us |
do_swap_page();
page_fault1_thr-1490 [023] 129.370419: funcgraph_entry:1.940us |
do_swap_page();
page_fault1_thr-1490 [023] 129.378847: funcgraph_entry: #1411.385 us |
do_swap_page();
page_fault1_thr-1490 [023] 129.380262: funcgraph_entry:3.916us |
do_swap_page();
page_fault1_thr-1490 [023] 129.380275: funcgraph_entry: #4287.751 us |
do_swap_page();
With the patch
runtest.py-1417 [020] 301.925911: funcgraph_entry: #9870.146 us |
do_swap_page();
runtest.py-1417 [020] 301.935785: funcgraph_entry:9.802us |
do_swap_page();
runtest.py-1417 [020] 301.935799: funcgraph_entry:3.551us |
do_swap_page();
runtest.py-1417 [020] 301.935806: funcgraph_entry:2.142us |
do_swap_page();
runtest.py-1417 [020] 301.935853: funcgraph_entry:6.938us |
do_swap_page();
runtest.py-1417 [020] 301.935864: funcgraph_entry:3.765us |
do_swap_page();
runtest.py-1417 [020] 301.935871: funcgraph_entry:3.600us |
do_swap_page();
runtest.py-1417 [020] 301.935878: funcgraph_entry:7.202us |
do_swap_page();
Acked-by: Tim Chen
Cc: Huang Ying
Cc: Minchan Kim
Signed-off-by: Yang Shi
---
v4: Added observed effects in the commit log per Andrew
v3: Move inode deference under swap device type check per Tim Chen
v2: Check the swap device type per Tim Chen
mm/swap_state.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/mm/swap_state.c b/mm/swap_state.c
index fd2f21e..78d500e 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -538,11 +538,18 @@ struct page *swap_cluster_readahead(swp_entry_t entry,
gfp_t gfp_mask,
bool do_poll = true, page_allocated;
struct vm_area_struct *vma = vmf->vma;
unsigned long addr = vmf->address;
+ struct inode *inode = NULL;
mask = swapin_nr_pages(offset) - 1;
if (!mask)
goto skip;
+ if (si->flags & (SWP_BLKDEV | SWP_FS)) {
+ inode = si->swap_file->f_mapping->host;
+ if (inode_read_congested(inode))
+ goto skip;
+ }
+
do_poll = false;
/* Read a page_cluster sized and aligned cluster around offset. */
start_offset = offset & ~mask;
--
1.8.3.1
[v4 PATCH 2/2] mm: swap: add comment for swap_vma_readahead
swap_vma_readahead()'s comment is missed, just add it.
Cc: Huang Ying
Cc: Tim Chen
Cc: Minchan Kim
Signed-off-by: Yang Shi
---
mm/swap_state.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 78d500e..dd8f698 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -698,6 +698,23 @@ static void swap_ra_info(struct vm_fault *vmf,
pte_unmap(orig_pte);
}
+/**
+ * swap_vm_readahead - swap in pages in hope we need them soon
+ * @entry: swap entry of this memory
+ * @gfp_mask: memory allocation flags
+ * @vmf: fault information
+ *
+ * Returns the struct page for entry and addr, after queueing swapin.
+ *
+ * Primitive swap readahead code. We simply read in a few pages whoes
+ * virtual addresses are around the fault address in the same vma.
+ *
+ * This has been extended to use the NUMA policies from the mm triggering
+ * the readahead.
+ *
+ * Caller must hold down_read on the vma->vm_mm if vmf->vma is not NULL.
+ *
+ */
static struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
struct vm_fault *vmf)
{
--
1.8.3.1
Re: [PATCH v8 00/25] Re-use nvram module
On Sun, 30 Dec 2018, I wrote: > > I'm not opposed to exported functions in place of a singleton ops > struct. Other things being equal I'm inclined toward the ops struct, > perhaps because I like encapsulation or perhaps because I don't like > excess generality. (That design decision was made years ago and I don't > remember the reasoning.) The rationale for the ops struct was that it offers introspection. It turns out that PPC64 device drivers don't care about byte-at-a-time accessors and X86 device drivers don't care about checksum validation. But that only gets us so far. We still needed a way to find out whether the arch has provided byte-at-a-time accessors (i.e. PPC32 and M68K Mac) or byte range accessors (i.e. PPC64 and those platforms with checksummed NVRAM like X86 and M68K Atari). You can't resolve this question at build time for a multi-platform kernel binary, so pre-processor tricks don't help. Device drivers tend to want to access NVRAM one byte at a time. With this patch series, those platforms which need checksum validation always set byte-at-a-time methods to NULL. (Hence the atari_scsi changes in patch 3.) The char misc driver is quite different to the usual device drivers, because the struct file_operations methods always access a byte range. The NULL methods in the ops struct allow the nvram.c misc device to avoid inefficient byte-at-a-time accessors where possible, just as arch/powerpc/kernel/nvram_64.c presently does. --
Re: + taint-fix-debugfs_simple_attrcocci-warnings.patch added to -mm tree
On (12/28/18 13:54), a...@linux-foundation.org wrote:
>
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
Looks OK to me.
Reviewed-by: Sergey Senozhatsky
> static __init int register_warn_debugfs(void)
> {
> /* Don't care about failure */
> - debugfs_create_file("clear_warn_once", 0200, NULL,
> - NULL, _warn_once_fops);
> + debugfs_create_file_unsafe("clear_warn_once", 0200, NULL, NULL,
> +_warn_once_fops);
> return 0;
> }
The commit message probably can be better.
The _unsafe() part suggests that some of them "safeness responsibilities"
are now panic.c responsibilities. The patch is OK since panic's
clear_warn_once_fops struct file_operations is safe against removal, so we
don't have to use otherwise necessary debugfs_file_get()/debugfs_file_put().
-ss
general protection fault in fdb_find_rcu
Hello, syzbot found the following crash on: HEAD commit:90cadbbf341d Merge git://git.kernel.org/pub/scm/linux/kern.. git tree: net-next console output: https://syzkaller.appspot.com/x/log.txt?x=1745c69b40 kernel config: https://syzkaller.appspot.com/x/.config?x=9d41c8529d7e7362 dashboard link: https://syzkaller.appspot.com/bug?extid=017b1f61c82a1c3e7efd compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15babaab40 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14c6142d40 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+017b1f61c82a1c3e7...@syzkaller.appspotmail.com IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready 8021q: adding VLAN 0 to HW filter on device team0 kasan: CONFIG_KASAN_INLINE enabled kasan: GPF could be caused by NULL-ptr deref or user memory access general protection fault: [#1] PREEMPT SMP KASAN CPU: 1 PID: 7824 Comm: syz-executor809 Not tainted 4.20.0-rc7+ #360 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:fdb_find_rcu+0x194/0xc90 include/linux/string.h:352 Code: 40 84 c6 0f 85 23 09 00 00 84 c9 0f 95 c2 0f 9e c0 84 c2 0f 85 13 09 00 00 48 b9 00 00 00 00 00 fc ff df 4c 89 e0 48 c1 e8 03 <0f> b6 14 08 49 8d 44 24 05 48 89 c6 48 c1 ee 03 0f b6 0c 0e 4c 89 RSP: 0018:8881b932f0b8 EFLAGS: 00010246 RAX: RBX: RCX: dc00 RDX: RSI: 111037265e01 RDI: 8881b932f1a8 RBP: 8881b932f310 R08: 8881bcc84400 R09: ed103b5a5b77 R10: ed103b5a5b77 R11: 8881dad2dbbb R12: R13: 0001 R14: 8881b9e68f40 R15: 8881b932f2e8 FS: 01e8e880() GS:8881dad0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 2180 CR3: 0001bec05000 CR4: 001406e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: br_fdb_find_rcu net/bridge/br_fdb.c:152 [inline] br_fdb_get+0xac/0x230 net/bridge/br_fdb.c:788 rtnl_fdb_get+0x8a0/0x13b0 net/core/rtnetlink.c:4166 rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:5125 netlink_rcv_skb+0x16c/0x430 net/netlink/af_netlink.c:2477 rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:5143 netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline] netlink_unicast+0x59f/0x750 net/netlink/af_netlink.c:1336 netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1917 sock_sendmsg_nosec net/socket.c:621 [inline] sock_sendmsg+0xd5/0x120 net/socket.c:631 ___sys_sendmsg+0x7fd/0x930 net/socket.c:2116 __sys_sendmsg+0x11d/0x280 net/socket.c:2154 __do_sys_sendmsg net/socket.c:2163 [inline] __se_sys_sendmsg net/socket.c:2161 [inline] __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x441679 Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 2b 09 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7fff99382768 EFLAGS: 0213 ORIG_RAX: 002e RAX: ffda RBX: 0003 RCX: 00441679 RDX: RSI: 2180 RDI: 0003 RBP: 006cd018 R08: 0100 R09: 0100 R10: 0020 R11: 0213 R12: 00402430 R13: 004024c0 R14: R15: Modules linked in: ---[ end trace d6d5286333ca98b5 ]--- RIP: 0010:fdb_find_rcu+0x194/0xc90 include/linux/string.h:352 Code: 40 84 c6 0f 85 23 09 00 00 84 c9 0f 95 c2 0f 9e c0 84 c2 0f 85 13 09 00 00 48 b9 00 00 00 00 00 fc ff df 4c 89 e0 48 c1 e8 03 <0f> b6 14 08 49 8d 44 24 05 48 89 c6 48 c1 ee 03 0f b6 0c 0e 4c 89 RSP: 0018:8881b932f0b8 EFLAGS: 00010246 RAX: RBX: RCX: dc00 RDX: RSI: 111037265e01 RDI: 8881b932f1a8 RBP: 8881b932f310 R08: 8881bcc84400 R09: ed103b5a5b77 R10: ed103b5a5b77 R11: 8881dad2dbbb R12: R13: 0001 R14: 8881b9e68f40 R15: 8881b932f2e8 FS: 01e8e880() GS:8881dad0() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 2180 CR3: 0001bec05000 CR4: 001406e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See:
Re: [PATCH] fanotify: allow freeze on suspend when waiting for response from userspace
On 12/29/18 3:34 PM, Orion Poplawski wrote:
On 12/29/18 3:04 PM, Orion Poplawski wrote:
On Thu 22-02-18 15:14:54, Kunal Shubham wrote:
>> On Fri 16-02-18 15:14:40, t.vi...@samsung.com wrote:
>> From: Vivek Trivedi
>> >> If fanotify userspace response server thread is frozen first,
>> it may fail to send response from userspace to kernel space
listener.
>> In this scenario, fanotify response listener will never get response
>> from userepace and fail to suspend.
>> >> Use freeze-friendly wait API to handle this issue.
>> >> Same problem was reported here:
>> https://bbs.archlinux.org/viewtopic.php?id=232270
>> >> Freezing of tasks failed after 20.005 seconds
>> (1 tasks refusing to freeze, wq_busy=0)
>> >> Backtrace:
>> [] (__schedule) from [] (schedule+0x4c/0xa4)
>> [] (schedule) from []
(fanotify_handle_event+0x1c8/0x218)
>> [] (fanotify_handle_event) from []
(fsnotify+0x17c/0x38c)
>> [] (fsnotify) from []
(security_file_open+0x88/0x8c)
>> [] (security_file_open) from []
(do_dentry_open+0xc0/0x338)
>> [] (do_dentry_open) from [] (vfs_open+0x54/0x58)
>> [] (vfs_open) from []
(do_last.isra.10+0x45c/0xcf8)
>> [] (do_last.isra.10) from []
(path_openat+0x424/0x600)
>> [] (path_openat) from []
(do_filp_open+0x3c/0x98)
>> [] (do_filp_open) from []
(do_sys_open+0x120/0x1e4)
>> [] (do_sys_open) from [] (SyS_open+0x28/0x2c)
>> [] (SyS_open) from []
(__sys_trace_return+0x0/0x20)
>
> Yeah, good catch.
>
>> @@ -63,7 +64,9 @@ static int fanotify_get_response(struct
fsnotify_group *group,
>> >> pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>> >> - wait_event(group->fanotify_data.access_waitq,
event->response);
>> + while (!event->response)
>> + wait_event_freezable(group->fanotify_data.access_waitq,
>> + event->response);
>
> But if the process gets a signal while waiting, we will just
livelock the
> kernel in this loop as wait_event_freezable() will keep returning
> ERESTARTSYS. So you need to be a bit more clever here...
Hi Jack,
Thanks for the quick review.
To avoid livelock issue, is it fine to use below change? If agree, I
will send v2 patch.
@@ -63,7 +64,11 @@ static int fanotify_get_response(struct
fsnotify_group *group,
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ if
(wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response))
+ flush_signals(current);
+ }
Hum, I don't think this is correct either as this way if any signal was
delivered while waiting for fanotify response, we'd just lose it while
previously it has been properly handled. So what I think needs to be
done
is that we just use wait_event_freezable() and propagate non-zero return
value (-ERESTARTSYS) up to the caller to handle the signal and
restart the
syscall as necessary.
Honza
--
Jan Kara
SUSE Labs, CR
Is there any progress here? This has become a real pain for us while
running BitDefender on EL7 laptops. I tried applying the following to
the EL7 kernel:
diff -up
linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
kernel-3.10.0-957.1.3.el7/linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
--- linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
2018-11-15 10:07:13.0 -0700
+++ linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
2018-12-28 15:44:26.452895337 -0700
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "fanotify.h"
@@ -64,7 +65,12 @@ static int fanotify_get_response(struct
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ ret =
wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response);
+ if (ret < 0)
+ return ret;
+ }
/* userspace responded, convert to something usable */
switch (event->response & ~FAN_AUDIT) {
but I get a kernel panic shortly after logging in to the system.
I tried a slightly different patch to see if setting event->response = 0
helps and to confirm the return value of wait_event_freezable:
--- linux-3.10.0-957.1.3.el7/fs/notify/fanotify/fanotify.c
2018-11-15 10:07:13.0 -0700
+++
linux-3.10.0-957.1.3.el7.fanotify.x86_64/fs/notify/fanotify/fanotify.c
2018-12-29 16:05:53.451125868 -0700
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "fanotify.h"
@@ -64,7 +65,15 @@
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ ret =
[PATCH] debugfs: debugfs_use_start/finish do not exist anymore
debugfs_use_file_start() and debugfs_use_file_finish() do not exist
since commit c9afbec27089 ("debugfs: purge obsolete SRCU based removal
protection"); tweak debugfs_create_file_unsafe() comment.
Signed-off-by: Sergey Senozhatsky
---
fs/debugfs/inode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 13b01351dd1c..4354069101b4 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -422,8 +422,8 @@ EXPORT_SYMBOL_GPL(debugfs_create_file);
* debugfs core.
*
* It is your responsibility to protect your struct file_operation
- * methods against file removals by means of debugfs_use_file_start()
- * and debugfs_use_file_finish(). ->open() is still protected by
+ * methods against file removals by means of debugfs_file_get()
+ * and debugfs_file_put(). ->open() is still protected by
* debugfs though.
*
* Any struct file_operations defined by means of
--
2.20.1
WARNING in wiphy_register (3)
Hello, syzbot found the following crash on: HEAD commit:90cadbbf341d Merge git://git.kernel.org/pub/scm/linux/kern.. git tree: net-next console output: https://syzkaller.appspot.com/x/log.txt?x=17053c9b40 kernel config: https://syzkaller.appspot.com/x/.config?x=9d41c8529d7e7362 dashboard link: https://syzkaller.appspot.com/bug?extid=73fd8b0aa60c67fa4b60 compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17211a5740 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+73fd8b0aa60c67fa4...@syzkaller.appspotmail.com IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 8021q: adding VLAN 0 to HW filter on device team0 netlink: 'syz-executor4': attribute type 23 has an invalid length. WARNING: CPU: 0 PID: 9553 at net/wireless/core.c:581 wiphy_verify_combinations net/wireless/core.c:581 [inline] WARNING: CPU: 0 PID: 9553 at net/wireless/core.c:581 wiphy_register+0x147e/0x28d0 net/wireless/core.c:784 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 9553 Comm: syz-executor4 Not tainted 4.20.0-rc7+ #360 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113 panic+0x2ad/0x55c kernel/panic.c:188 __warn.cold.8+0x20/0x45 kernel/panic.c:540 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:290 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973 RIP: 0010:wiphy_verify_combinations net/wireless/core.c:581 [inline] RIP: 0010:wiphy_register+0x147e/0x28d0 net/wireless/core.c:784 Code: f0 ff ff e8 74 30 2b fa 0f 0b bb ea ff ff ff e9 10 f0 ff ff e8 63 30 2b fa 0f 0b bb ea ff ff ff e9 ff ef ff ff e8 52 30 2b fa <0f> 0b bb ea ff ff ff e9 ee ef ff ff e8 41 30 2b fa 0f 0b bb ea ff kobject: 'loop1' (a0078c7e): kobject_uevent_env RSP: 0018:8881d798edb0 EFLAGS: 00010293 RAX: 8881cd55e040 RBX: 0001 RCX: 8753236b RDX: RSI: 8753292e RDI: 0001 RBP: 8881d798ef38 R08: 8881cd55e040 R09: ed103a4a0680 R10: ed103a4a0680 R11: 8881d2503405 R12: 002f R13: 88f0ba60 R14: 8881d2505e74 R15: kobject: 'loop1' (a0078c7e): fill_kobj_path: path = '/devices/virtual/block/loop1' ieee80211_register_hw+0x15cd/0x40e0 net/mac80211/main.c:1109 mac80211_hwsim_new_radio+0x2025/0x3630 drivers/net/wireless/mac80211_hwsim.c:2921 kobject: 'loop5' (3dbebf8f): kobject_uevent_env kobject: 'loop5' (3dbebf8f): fill_kobj_path: path = '/devices/virtual/block/loop5' hwsim_new_radio_nl+0xd3a/0x14b0 drivers/net/wireless/mac80211_hwsim.c:3469 genl_family_rcv_msg+0x8a7/0x11a0 net/netlink/genetlink.c:601 genl_rcv_msg+0xc6/0x168 net/netlink/genetlink.c:626 netlink_rcv_skb+0x16c/0x430 net/netlink/af_netlink.c:2477 genl_rcv+0x28/0x40 net/netlink/genetlink.c:637 netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline] netlink_unicast+0x59f/0x750 net/netlink/af_netlink.c:1336 netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1917 sock_sendmsg_nosec net/socket.c:621 [inline] sock_sendmsg+0xd5/0x120 net/socket.c:631 ___sys_sendmsg+0x7fd/0x930 net/socket.c:2116 __sys_sendmsg+0x11d/0x280 net/socket.c:2154 __do_sys_sendmsg net/socket.c:2163 [inline] __se_sys_sendmsg net/socket.c:2161 [inline] __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457759 Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f3a2a64fc78 EFLAGS: 0246 ORIG_RAX: 002e RAX: ffda RBX: 0003 RCX: 00457759 RDX: RSI: 2080 RDI: 0003 RBP: 0073bf00 R08: R09: R10: R11: 0246 R12: 7f3a2a6506d4 R13: 004c49c9 R14: 004d80a0 R15: Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Re: tpm_tis TPM2.0 not detected on cold boot
On Tue, 2018-12-25 at 14:55 +0100, Michael Niewöhner wrote: > On Sun, 2018-12-23 at 12:55 +0100, Michael Niewöhner wrote: > > Hi Mimi, > > > > On Sat, 2018-12-22 at 17:53 -0500, Mimi Zohar wrote: > > > On Sat, 2018-12-22 at 14:47 +0100, Michael Niewöhner wrote: > > > > > > > When I remove the timeout and boot directly to the linux kernel, I get > > > > that > > > > "2314 TPM-self test error" since it has not finished, yet. The TPM is > > > > detected > > > > by IMA and works fine then. > > > > > > > > Some more tests showed that any delay before booting the kernel causes > > > > the > > > > TPM > > > > to not get detected. I tested, 10, 15, 20, 30, 60... seconds. Only in > > > > some > > > > very > > > > rare cases the TPM got detected. > > > > > > > > I wanted to know if the TPM is in an well initialized state at the time > > > > of > > > > that > > > > error. Since I was not able to get some test/debug kernel patches > > > > working > > > > I > > > > decided to try kexec. It turned out that the TPM is indeed correctly > > > > working > > > > and > > > > will be detected just fine by linux after kexec! > > > > > > No surprise here. kexec would be the equivalent of a soft reboot. > > > > Well, I am not that deep in kexec internals but isn't a soft reboot much > > more > > than a kexec? I thought kexec would "just" load the new kernel to memory and > > executes it while a soft reboot goes at least through some UEFI > > initialization. > > For example, my pwm fans - in fact the EC - get resetted on a soft reboot, > > while > > a kexec does not touch them. > > Similarly, the PCRs are not reset on kexec. > > That is why I wanted to test if there is a different behaviour on kexec > > compared > > to a "real" soft reboot. If there was such difference I would have assumed a > > UEFI bug that does not initialize the TPM correctly. > > Kexec AFAIK does not invoke any UEFI initialization, so the TPM should be in > > the > > same state as before kexec and since there is no difference between sr and > > kexec > > I have the feeling there is something wrong in the kernel. > > > > Correct me if I am wrong here, please. But the problem you've described is on a cold boot, not a soft reboot. Both the soft reboot and kexec are working properly. It seems the difference is that on a cold boot, the TPM takes longer to initialize. > > My current workaround is to do a machine_emergency_reboot() when TPM isn't > > detected correctly. That is a pretty hard workaround but it seems to work > > for > > now... This is a again soft reboot. > > > > > > > > > > > > Is there anyone having an idea what could be wrong here? I am willing to > > > > debug > > > > this but I have really no idea where to start :-( > > > > > > A while ago, I was "playing" with a pi. Commenting out > > > tpm2_do_selftest() seemed to resolve a similar problem, but that was > > > before James' patches. I don't know if that would make a difference > > > now. > > > > Hm, I will try that.. > > > > Unfortunately this did not change anything Not much I can do now. After vacation, I'll set up the pi to see if it is working properly with a recent kernel. Mimi
Re: [PATCH v8 24/25] powerpc: Adopt nvram module for PPC64
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
> On Wed, Dec 26, 2018 at 1:43 AM Finn Thain wrote:
>
> > +static ssize_t ppc_nvram_get_size(void)
> > +{
> > + if (ppc_md.nvram_size)
> > + return ppc_md.nvram_size();
> > + return -ENODEV;
> > +}
>
> > +const struct nvram_ops arch_nvram_ops = {
> > + .read = ppc_nvram_read,
> > + .write = ppc_nvram_write,
> > + .get_size = ppc_nvram_get_size,
> > + .sync = ppc_nvram_sync,
> > +};
>
> Coming back to this after my comment on the m68k side, I notice that
> there is now a double indirection through function pointers. Have you
> considered completely removing the operations from ppc_md instead by
> having multiple copies of nvram_ops?
>
I considered a few alternatives. I figured that it was refactoring that
could be deferred, as it would be confined to arch/powerpc. I was more
interested in the cross-platform API.
> With the current method, it does seem odd to have a single
> per-architecture instance of the exported structure containing function
> pointers. This doesn't give us the flexibility of having multiple copies
> in the kernel the way that ppc_md does, but it adds overhead compared to
> simply exporting the functions directly.
>
You're right, there is overhead here.
With a bit of auditing, wrappers like the one you quoted (which merely
checks whether or not a ppc_md method is implemented) could surely be
avoided.
The arch_nvram_ops methods are supposed to optional (that is, they are
allowed to be NULL).
We could call exactly the same function pointers though either ppc_md or
arch_nvram_ops. That would avoid the double indirection.
--
>Arnd
>
[PATCH] Fix compilation problem for mt7621_wdt.c and rt2880_wdt.c
These files need
#include
to compile correctly.
Fixes: ac3167257b9f ("headers: separate linux/mod_devicetable.h from
linux/platform_device.h")
Signed-off-by: NeilBrown
---
drivers/watchdog/mt7621_wdt.c | 1 +
drivers/watchdog/rt2880_wdt.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/watchdog/mt7621_wdt.c b/drivers/watchdog/mt7621_wdt.c
index 5c4a764717c4..81208cd3f4ec 100644
--- a/drivers/watchdog/mt7621_wdt.c
+++ b/drivers/watchdog/mt7621_wdt.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
diff --git a/drivers/watchdog/rt2880_wdt.c b/drivers/watchdog/rt2880_wdt.c
index 98967f0a7d10..db7c57d82cfd 100644
--- a/drivers/watchdog/rt2880_wdt.c
+++ b/drivers/watchdog/rt2880_wdt.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
--
2.14.0.rc0.dirty
signature.asc
Description: PGP signature
[PATCH] staging: comedi: cb_pcimdas.c: fixed an alignment coding style issue
Fixed a coding style issue.
Signed-off-by: William Mitchell Jr
---
drivers/staging/comedi/drivers/cb_pcimdas.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcimdas.c
b/drivers/staging/comedi/drivers/cb_pcimdas.c
index 4e72a0778086..a9d052bfda38 100644
--- a/drivers/staging/comedi/drivers/cb_pcimdas.c
+++ b/drivers/staging/comedi/drivers/cb_pcimdas.c
@@ -252,9 +252,9 @@ static int cb_pcimdas_di_insn_bits(struct comedi_device
*dev,
}
static int cb_pcimdas_do_insn_bits(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
{
struct cb_pcimdas_private *devpriv = dev->private;
--
2.20.1
Re: 4.14 backport request for dbdda842fe96f: "printk: Add console owner and waiter logic to load balance console writes"
On (12/28/18 16:03), Daniel Wang wrote: > Thanks. I was able to confirm that commit c7c3f05e341a9a2bd alone > fixed the problem for me. As expected, all 16 CPUs' stacktrace was > printed, before a final panic stack dump and a successful reboot. Cool, thanks! -ss
Re: [GIT PULL] security: general updates for v4.21
On Sat, 2018-12-29 at 10:34 -0800, Casey Schaufler wrote: > On 12/28/2018 8:15 PM, Linus Torvalds wrote: > > On Fri, Dec 28, 2018 at 8:09 PM James Morris wrote: > >> Yep, I understand what you mean. I can't find the discussion from several > >> years ago, but developers asked to be able to work with more current > >> kernels, and I recall you saying that if you want to do this, merge to a > >> specific -rc tag at least. > > If people really want it, maybe the merge can state that explicit > > thing, as it is I'm trying to push back on empty merges that don't > > explain why they even exist. > > > > Linus > > The security tree tends to get changed from multiple directions, > most of which aren't based out of the security sub-system. The mount > rework from David is an excellent example. It gets hit just about > any time there's a significant VFS or networking change. Keeping > it current has helped find issues much earlier in the process. Agreed, the security subsystem is different than other subsystems. In addition to VFS changes, are key changes. Changes in other subsystems do affect the LSMs/integrity. Included in this open window are a number of LSM changes, which were not posted on the LSM mailing list and are not being upstreamed via the LSMs. Mimi
Re: KASAN: use-after-free Read in ax25_fillin_cb
Hi, Joerg On Sat, Dec 29, 2018 at 2:06 PM Joerg Reuter wrote: > Unfortunately, I'm on a low bandwidth connection right now. I'd be > grateful if someone could create a patch. This is likely not a high > impact issue (unpriviliged users can't set up or tear down interfaces), > still it may cause hard to find memory corruptions. I already sent a patch: http://patchwork.ozlabs.org/patch/1019386/ Sorry for forgetting to Cc more people. Thanks.
Re: linux-next: manual merge of the rdma tree with the nfs-anna tree
Hi all,
On Mon, 24 Dec 2018 14:13:52 +1100 Stephen Rothwell
wrote:
>
> Today's linux-next merge of the rdma tree got a conflict in:
>
> net/sunrpc/xprtrdma/fmr_ops.c
>
> between commit:
>
> d31f8deac38d ("xprtrdma: Remove support for FMR memory registration")
>
> from the nfs-anna tree and commit:
>
> 3023a1e93656 ("RDMA: Start use ib_device_ops")
>
> from the rdma tree.
>
> I fixed it up (the former removed the file, so I did that) and can
> carry the fix as necessary. This is now fixed as far as linux-next is
> concerned, but any non trivial conflicts should be mentioned to your
> upstream maintainer when your tree is submitted for merging. You may
> also want to consider cooperating with the maintainer of the conflicting
> tree to minimise any particularly complex conflicts.
This is now a conflict between the nfs-anna tree and Linus' tree.
--
Cheers,
Stephen Rothwell
pgp7nMC1Tvuu3.pgp
Description: OpenPGP digital signature
[PATCH v5 1/2] drm/amd: validate user pitch alignment
Userspace may request pitch alignment that is not supported by GPU.
Some requests 32, but GPU ignores it and uses default 64 when cpp is
4. If GEM object is allocated based on the smaller alignment, GPU
DMA will go out of bound.
Cc: sta...@vger.kernel.org # v4.2+
Signed-off-by: Yu Zhao
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 16
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 15ce7e681d67..16af80ccd0a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -527,6 +527,22 @@ amdgpu_display_user_framebuffer_create(struct drm_device
*dev,
struct drm_gem_object *obj;
struct amdgpu_framebuffer *amdgpu_fb;
int ret;
+ struct amdgpu_device *adev = dev->dev_private;
+ int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
+ int pitch = mode_cmd->pitches[0] / cpp;
+
+ if (pitch < mode_cmd->width) {
+ DRM_DEBUG_KMS("Expecting pitch(%d)/cpp(%d) >= width(%d)\n",
+ mode_cmd->pitches[0], cpp, mode_cmd->width);
+ return ERR_PTR(-EINVAL);
+ }
+
+ pitch = amdgpu_align_pitch(adev, pitch, cpp, false);
+ if (mode_cmd->pitches[0] != pitch) {
+ DRM_DEBUG_KMS("Invalid pitch: expecting %d but got %d\n",
+ pitch, mode_cmd->pitches[0]);
+ return ERR_PTR(-EINVAL);
+ }
obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[0]);
if (obj == NULL) {
--
2.20.1.415.g653613c723-goog
[PATCH v5 2/2] drm/amd: validate user GEM object size
When creating frame buffer, userspace may request to attach to a
previously allocated GEM object that is smaller than what GPU
requires. Validation must be done to prevent out-of-bound DMA,
otherwise it could be exploited to reveal sensitive data.
This fix is not done in a common code path because individual
driver might have different requirement.
Cc: sta...@vger.kernel.org # v4.2+
Signed-off-by: Yu Zhao
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 16af80ccd0a0..61c075a78ee1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -527,6 +527,7 @@ amdgpu_display_user_framebuffer_create(struct drm_device
*dev,
struct drm_gem_object *obj;
struct amdgpu_framebuffer *amdgpu_fb;
int ret;
+ int height;
struct amdgpu_device *adev = dev->dev_private;
int cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);
int pitch = mode_cmd->pitches[0] / cpp;
@@ -557,6 +558,13 @@ amdgpu_display_user_framebuffer_create(struct drm_device
*dev,
return ERR_PTR(-EINVAL);
}
+ height = ALIGN(mode_cmd->height, 8);
+ if (obj->size < pitch * height) {
+ DRM_DEBUG_KMS("Invalid GEM size: expecting >= %d but got %zu\n",
+ pitch * height, obj->size);
+ return ERR_PTR(-EINVAL);
+ }
+
amdgpu_fb = kzalloc(sizeof(*amdgpu_fb), GFP_KERNEL);
if (amdgpu_fb == NULL) {
drm_gem_object_put_unlocked(obj);
--
2.20.1.415.g653613c723-goog
Re: [PATCH v8 00/25] Re-use nvram module
On Sat, 29 Dec 2018, Arnd Bergmann wrote:
> I had a look at the complete series now, and I think this is a great
> cleanup. I replied with a couple of minor comments that you may or may
> not want to address first.
>
Thanks for reviewing this.
> The one thing I would like to see resolved (I hope this doesn't bring
> back an old discussion you had already concluded) is regarding the use
> of a global exported structure of function pointers, as opposed to using
> either directly exported functions (with a consistent interface) or a
> boot-time selectable structure like dma_map_ops or ppc_md.
>
If I understand correctly, /dev/nvram was made obsolete by the nvmem
subsystem (?). If so, there won't be new /dev/nvram users, and the
refactoring here only has to be sufficiently flexible to meet the needs of
existing users.
I'm not opposed to exported functions in place of a singleton ops struct.
Other things being equal I'm inclined toward the ops struct, perhaps
because I like encapsulation or perhaps because I don't like excess
generality. (That design decision was made years ago and I don't remember
the reasoning.)
All the arch_nvram_ops structs that I've defined in these patches have the
'const' properly:
const struct nvram_ops arch_nvram_ops = {
.read_byte = nvram_read_byte,
.write_byte = nvram_write_byte,
.read = nvram_read,
.write = nvram_write,
.get_size = nvram_get_size,
.set_checksum = nvram_set_checksum,
.initialize = nvram_initialize,
};
EXPORT_SYMBOL(arch_nvram_ops);
This is because there's no need to do any run-time reconfiguration.
Is a collection of exported functions a better fit here?
--
> Arnd
>
Re: [PATCH] Staging: vt6655: Fix camel case of variable
On Sat, 2018-12-29 at 23:59 +0100, Petr Sedlák wrote:
> Replace variable uDelayUnit with u_delay_unit. Issue found by
> checkpatch.
probably better as a static inline too.
> diff --git a/drivers/staging/vt6655/upc.h b/drivers/staging/vt6655/upc.h
[]
> @@ -42,15 +42,15 @@
> #define VNSvOutPortD(dwIOAddress, dwData) \
> iowrite32((u32)(dwData), dwIOAddress)
>
> -#define PCAvDelayByIO(uDelayUnit)\
> +#define PCAvDelayByIO(u_delay_unit) \
> do { \
> unsigned char byData; \
> unsigned long ii; \
> \
> - if (uDelayUnit <= 50) { \
> - udelay(uDelayUnit); \
> + if (u_delay_unit <= 50) { \
> + udelay(u_delay_unit); \
> } else {\
> - for (ii = 0; ii < (uDelayUnit); ii++) \
> + for (ii = 0; ii < (u_delay_unit); ii++) \
> byData = inb(0x61); \
> } \
> } while (0)
And as the thing is used only once with a #define
with a value less than 50, maybe just
udelay(CB_DELAY_LOOP_WAIT);
in that one place.
Re: [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte()
On Sat, 29 Dec 2018, LEROY Christophe wrote:
> Finn Thain a ?crit?:
>
> > Make use of arch_nvram_ops in device drivers so that the nvram_* function
> > exports can be removed.
> >
> > Since they are no longer global symbols, rename the PPC32 nvram_* functions
> > appropriately.
> >
> > Signed-off-by: Finn Thain
> > ---
> > arch/powerpc/kernel/setup_32.c | 8
> > drivers/char/generic_nvram.c | 4 ++--
> > drivers/video/fbdev/controlfb.c| 4 ++--
> > drivers/video/fbdev/imsttfb.c | 4 ++--
> > drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
> > drivers/video/fbdev/platinumfb.c | 4 ++--
> > drivers/video/fbdev/valkyriefb.c | 4 ++--
> > 7 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
> > index e0d045677472..bdbe6acbef11 100644
> > --- a/arch/powerpc/kernel/setup_32.c
> > +++ b/arch/powerpc/kernel/setup_32.c
> > @@ -152,20 +152,18 @@ __setup("l3cr=", ppc_setup_l3cr);
> >
> > #ifdef CONFIG_GENERIC_NVRAM
> >
> > -unsigned char nvram_read_byte(int addr)
> > +static unsigned char ppc_nvram_read_byte(int addr)
> > {
> > if (ppc_md.nvram_read_val)
> > return ppc_md.nvram_read_val(addr);
> > return 0xff;
> > }
> > -EXPORT_SYMBOL(nvram_read_byte);
> >
> > -void nvram_write_byte(unsigned char val, int addr)
> > +static void ppc_nvram_write_byte(unsigned char val, int addr)
> > {
> > if (ppc_md.nvram_write_val)
> > ppc_md.nvram_write_val(addr, val);
> > }
> > -EXPORT_SYMBOL(nvram_write_byte);
> >
> > static ssize_t ppc_nvram_get_size(void)
> > {
> > @@ -182,6 +180,8 @@ static long ppc_nvram_sync(void)
> > }
> >
> > const struct nvram_ops arch_nvram_ops = {
> > + .read_byte = ppc_nvram_read_byte,
> > + .write_byte = ppc_nvram_write_byte,
> > .get_size = ppc_nvram_get_size,
> > .sync = ppc_nvram_sync,
> > };
> > diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
> > index f32d5663de95..41b76bf9614e 100644
> > --- a/drivers/char/generic_nvram.c
> > +++ b/drivers/char/generic_nvram.c
> > @@ -48,7 +48,7 @@ static ssize_t read_nvram(struct file *file, char __user
> > *buf,
> > if (*ppos >= nvram_len)
> > return 0;
> > for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
> > - if (__put_user(nvram_read_byte(i), p))
> > + if (__put_user(arch_nvram_ops.read_byte(i), p))
>
> Instead of modifying all drivers (in this patch and previous ones related to
> other arches), wouldn't it be better to add helpers like the following in
> nvram.h:
>
> Static inline unsigned char nvram_read_byte(int addr)
> {
>return arch_nvram_ops.read_byte(addr);
> }
>
Is there some benefit, or is that just personal taste?
Avoiding changes to call sites avoids code review, but I think 1) the
thinkpad_acpi changes have already been reviewed and 2) the fbdev changes
need review anyway.
Your suggesion would add several new entities and one extra layer of
indirection.
I think indirection harms readability because now the reader now has to go
and look up the meaning of the new entities.
It's not the case that we need to choose between definitions of
nvram_read_byte() at compile time, or stub them out:
#ifdef CONFIG_FOO
static inline unsigned char nvram_read_byte(int addr)
{
return arch_nvram_ops.read_byte(addr);
}
#else
static inline unsigned char nvram_read_byte(int addr) { }
#endif
And I don't anticipate a need for a macro here either:
#define nvram_read_byte(a) random_nvram_read_byte_impl(a)
I think I've used the simplest solution.
--
> Christophe
>
Re: [RFC PATCH] media: rcar-vin: Allow independent VIN link enablement
Hi Niklas,
On 12/26/18 4:51 PM, Niklas Söderlund wrote:
Hi Steve,
Thanks for your patch.
On 2018-12-25 15:27:25 -0800, Steve Longerbeam wrote:
There is a block of code in rvin_group_link_notify() that prevents
enabling a link to a VIN node if any entity in the media graph is
in use. This prevents enabling a VIN link even if there is an in-use
entity somewhere in the graph that is independent of the link's
pipeline.
For example, the code block will prevent enabling a link from
the first rcar-csi2 receiver to a VIN node even if there is an
enabled link somewhere far upstream on the second independent
rcar-csi2 receiver pipeline.
Unfortunately this is by design and needed due to the hardware design.
The different VIN endpoints shares a configuration register which
controls the routing from the CSI-2 receivers to the VIN (register name
CHSEL). Modifying the CHSEL register which is what happens when a link
is enabled/disabled can have side effects on running streams even if
they are not shown to be dependent in the media graph.
Ok, understood, modifying CHSEL register can adversely affect running
streams.
There is a CHSEL register in VIN0 which controls the routing from all
CSI-2 receivers to VIN0-3 and a CHSEL register in VIN4 which controls
the same for VIN4-7.
If this code block is meant to prevent modifying a link if the
link is actively involved in streaming, there is already such a
check in __media_entity_setup_link() that verifies the stream_count
of the link's source and sink entities are both zero.
For the reason above the check in __media_entity_setup_link() is not
enough :-( This register sharing is my least favorite thing about the
VIN on Gen3 and forces the driver to become more complex as all VIN
instances needs to know about each other and interact.
Given above I understand why the stream count checks in
__media_entity_setup_link() are insufficient, because only the requested
link's source stream count is checked, and not the other CSI-2 receiver
for example.
But why check the use counts of every entity upstream from the VIN
sources? Why not check only the VIN source entities stream counts (both
CSI-2 receivers and/or parallel devices), and ignore entities upstream
from those?
And why are the use counts checked, it seems it should be the stream
counts that should be checked.
Remove the code block so that VIN node links can be enabled even if
there are other independent in-use entities.
There is room for some improvement in this area disregarding the odd
hardware design. It *could* be allowed to change a link terminating in
VIN4-7 even if there is a stream running for one or more in VIN0-3.
I would be interested to test such a patch but to allow any link change
which is allowed by __media_entity_setup_link() is unfortunately not
possible, as I understand it. Maybe someone more clever then me can find
ways to unlock even more then just the split between VIN0-3 and VIn4-7.
In essence the CHSEL register can not be changed if it's involved in a
running pipeline even if the end result would be that the running
pipeline would look the same. This is possible as there are multiple
CHSEL settings where the same source is connected to a specific VIN
while other members of the "subgroup of VINs" (e.g. VIN0-3) is routed to
something else for the two CHSEL settings.
Right, so rvin_group_link_notify() determines whether the requested VIN
link enable will result in a valid set of CSI2->VIN links for the given
hardware, using the CHSEL bitmask tables. Which is why it seems it is
the stream counts that should be checked as mentioned above, rather than
the use counts, because the CHSEL bitmask checks are validating the set
of enabled links, and the only remaining checks are to verify no streams
are running on either CSI-2 receiver.
Steve
Fixes: c0cc5aef31 ("media: rcar-vin: add link notify for Gen3")
Signed-off-by: Steve Longerbeam
---
drivers/media/platform/rcar-vin/rcar-core.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c
b/drivers/media/platform/rcar-vin/rcar-core.c
index f0719ce24b97..b2c9a876969e 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -116,7 +116,6 @@ static int rvin_group_link_notify(struct media_link *link,
u32 flags,
struct rvin_group, mdev);
unsigned int master_id, channel, mask_new, i;
unsigned int mask = ~0;
- struct media_entity *entity;
struct video_device *vdev;
struct media_pad *csi_pad;
struct rvin_dev *vin = NULL;
@@ -131,11 +130,6 @@ static int rvin_group_link_notify(struct media_link *link,
u32 flags,
!is_media_entity_v4l2_video_device(link->sink->entity))
return 0;
- /* If any entity is in use don't allow link changes. */
- media_device_for_each_entity(entity, >mdev)
-
Re: [PATCH AUTOSEL 4.14 25/35] iomap: sub-block dio needs to zeroout beyond EOF
On Fri, Dec 28, 2018 at 09:06:24AM +0100, Pavel Machek wrote: > On Mon 2018-12-03 23:22:46, Thomas Backlund wrote: > > Den 2018-12-03 kl. 11:22, skrev Sasha Levin: > > > > > > > > This is a case where theory collides with the real world. Yes, our QA is > > > lacking, but we don't have the option of not doing the current process. > > > If we stop backporting until a future data where our QA problem is > > > solved we'll end up with what we had before: users stuck on ancient > > > kernels without a way to upgrade. > > > > > > > Sorry, but you seem to be living in a different "real world"... > > I have to agree here :-(. > > > People stay on "ancient kernels" that "just works" instead of updating > > to a newer one that "hopefully/maybe/... works" > > Stable has a rules community agreed on, unfortunately stable team just > simply ignores those and decided to do "whatever they please". > > Process went from "serious bugs that bother people only" to "hey, this > looks like a bugfix, lets put it into tree and see what it breaks"... Resulting in us having to tell users not to use stable kernels because they can contain broken commits from upstream that did not go through maintainer tree and test cycles. https://marc.info/?l=linux-xfs=154544499507105=2 In this case, the broken commit to the fs/iomap.c code was merged upstream through the akpm tree, rather than the XFS tree and test process as previous changes to this code had been staged. It was then backported so fast and released so quickly that it hadn't got back into the XFS upstream tree test cycles until after it had already committed to at least one stable kernel. We'd only just registered and confirmed a regression in in post -rc7 upstream trees when the stale kernel containing the commit was released. It took us another couple of days to isolate failing configuration and bisect it down to the commit. Only when I got "formlettered" for cc'ing the stable kernel list on the revert patch (because I wanted to make sure the stable kernel maintainers knew it was being reverted and so it wouldn't be backported) did I learn it had already been "auto-backported" and released in a stable kernel in under a week. Essentially, the "auto-backport" completely short-circuited the upstream QA process. IOWs, if you were looking for a case study to demonstrate the failings of the current stable process, this is it. Cheers, Dave. -- Dave Chinner da...@fromorbit.com
WARNING in mmu_spte_clear_track_bits (2)
Hello, syzbot found the following crash on: HEAD commit:6a1d293238c1 Add linux-next specific files for 20181224 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=131ae9ed40 kernel config: https://syzkaller.appspot.com/x/.config?x=f9369d117d073843 dashboard link: https://syzkaller.appspot.com/bug?extid=9aaa207a0b90b704eeda compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15a6629b40 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11a2d29b40 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+9aaa207a0b90b704e...@syzkaller.appspotmail.com sshd (7857) used greatest stack depth: 15720 bytes left L1TF CPU bug present and SMT on, data leak possible. See CVE-2018-3646 and https://www.kernel.org/doc/html/latest/admin-guide/l1tf.html for details. WARNING: CPU: 0 PID: 7873 at arch/x86/kvm/mmu.c:830 mmu_spte_clear_track_bits+0x45f/0x520 arch/x86/kvm/mmu.c:830 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 7873 Comm: syz-executor226 Not tainted 4.20.0-rc7-next-20181224 #188 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113 panic+0x2ad/0x632 kernel/panic.c:214 __warn.cold.8+0x20/0x4f kernel/panic.c:571 report_bug+0x254/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:176 [inline] do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:269 do_invalid_op+0x36/0x40 arch/x86/kernel/traps.c:288 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973 RIP: 0010:mmu_spte_clear_track_bits+0x45f/0x520 arch/x86/kvm/mmu.c:830 Code: 40 ff ff ff 31 ff 44 89 fe 48 ba 00 00 00 00 00 fc ff df c6 04 10 f8 e8 4f ad 70 00 45 85 ff 0f 85 65 fd ff ff e8 31 ac 70 00 <0f> 0b e9 59 fd ff ff e8 25 ac 70 00 4c 89 ef e8 4d 33 f7 ff 31 f6 RSP: 0018:8881b9df6860 EFLAGS: 00010293 RAX: 8881b1b6c3c0 RBX: 4001af5e8c77 RCX: 81114481 RDX: RSI: 8111448f RDI: 0005 RBP: 8881b9df6978 R08: 8881b1b6c3c0 R09: f94000d7af46 R10: f94000d7af46 R11: ea0006bd7a37 R12: 1110373bed0e R13: 001af5e8 R14: 8881b9df6950 R15: drop_spte+0x24/0x220 arch/x86/kvm/mmu.c:1468 mmu_page_zap_pte+0x2d3/0x3a0 arch/x86/kvm/mmu.c:2613 kvm_mmu_page_unlink_children arch/x86/kvm/mmu.c:2635 [inline] kvm_mmu_prepare_zap_page+0x1f9/0x1530 arch/x86/kvm/mmu.c:2679 kvm_zap_obsolete_pages arch/x86/kvm/mmu.c:5843 [inline] kvm_mmu_invalidate_zap_all_pages+0x348/0x7b0 arch/x86/kvm/mmu.c:5884 kvm_arch_flush_shadow_all+0x15/0x20 arch/x86/kvm/x86.c:9431 kvm_mmu_notifier_release+0x59/0x90 arch/x86/kvm/../../../virt/kvm/kvm_main.c:494 __mmu_notifier_release+0x1e9/0x630 mm/mmu_notifier.c:68 mmu_notifier_release include/linux/mmu_notifier.h:261 [inline] exit_mmap+0xa1/0x590 mm/mmap.c:3088 __mmput kernel/fork.c:1045 [inline] mmput+0x247/0x610 kernel/fork.c:1066 exit_mm kernel/exit.c:545 [inline] do_exit+0xdcc/0x25f0 kernel/exit.c:854 do_group_exit+0x177/0x440 kernel/exit.c:970 get_signal+0x8b0/0x1980 kernel/signal.c:2515 do_signal+0x9c/0x21c0 arch/x86/kernel/signal.c:816 exit_to_usermode_loop+0x2e5/0x380 arch/x86/entry/common.c:162 prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline] syscall_return_slowpath arch/x86/entry/common.c:268 [inline] do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x448c79 Code: e8 3c af 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 4b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7fe328b1fcf8 EFLAGS: 0246 ORIG_RAX: 00ca RAX: fe00 RBX: 006ddc58 RCX: 00448c79 RDX: RSI: 0080 RDI: 006ddc58 RBP: 006ddc50 R08: R09: R10: R11: 0246 R12: 006ddc5c R13: 7ffcaa19611f R14: 7fe328b209c0 R15: 006ddd4c Kernel Offset: disabled Rebooting in 86400 seconds.. --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
[PATCH] Staging: vt6655: Fix camel case of variable
Replace variable uDelayUnit with u_delay_unit. Issue found by
checkpatch.
Signed-off-by: Petr Sedlák
---
drivers/staging/vt6655/upc.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/vt6655/upc.h b/drivers/staging/vt6655/upc.h
index 61b3e568ff9a..384af225336a 100644
--- a/drivers/staging/vt6655/upc.h
+++ b/drivers/staging/vt6655/upc.h
@@ -42,15 +42,15 @@
#define VNSvOutPortD(dwIOAddress, dwData) \
iowrite32((u32)(dwData), dwIOAddress)
-#define PCAvDelayByIO(uDelayUnit) \
+#define PCAvDelayByIO(u_delay_unit)\
do { \
unsigned char byData; \
unsigned long ii; \
\
- if (uDelayUnit <= 50) { \
- udelay(uDelayUnit); \
+ if (u_delay_unit <= 50) { \
+ udelay(u_delay_unit); \
} else {\
- for (ii = 0; ii < (uDelayUnit); ii++) \
+ for (ii = 0; ii < (u_delay_unit); ii++) \
byData = inb(0x61); \
} \
} while (0)
--
2.17.1
Re: [PATCH v8 00/25] Re-use nvram module
On Wed, Dec 26, 2018 at 1:43 AM Finn Thain wrote: > This allows for removal of drivers/char/generic_nvram.c as well as some > duplicated code in arch/powerpc/kernel/nvram_64.c. By reducing the number > of /dev/nvram char misc device implementations, the number of bugs and > inconsistencies is also reduced. > > This patch series reduces inconsistencies between PPC32 and PPC64, and > between PPC_PMAC and MAC. A uniform API has benefits for userspace. > For example, some error codes for some ioctl calls become consistent > across PowerPC platforms. The uniform API can potentially benefit > bootloaders that work across different platforms which all have XPRAM > (e.g. Emile). > > I think there are two reasonable merge strategies for this patch series. > The char misc maintainer could take the entire series. Alternatively the > m68k maintainer could take patches 1 thru 14, and after those patches > reach mainline the powerpc maintainer could take 15 thru 25 (even though > patch 21 is not powerpc-related). I had a look at the complete series now, and I think this is a great cleanup. I replied with a couple of minor comments that you may or may not want to address first. The one thing I would like to see resolved (I hope this doesn't bring back an old discussion you had already concluded) is regarding the use of a global exported structure of function pointers, as opposed to using either directly exported functions (with a consistent interface) or a boot-time selectable structure like dma_map_ops or ppc_md. Arnd
Re: [PATCH v8 24/25] powerpc: Adopt nvram module for PPC64
On Wed, Dec 26, 2018 at 1:43 AM Finn Thain wrote:
> +static ssize_t ppc_nvram_get_size(void)
> +{
> + if (ppc_md.nvram_size)
> + return ppc_md.nvram_size();
> + return -ENODEV;
> +}
> +const struct nvram_ops arch_nvram_ops = {
> + .read = ppc_nvram_read,
> + .write = ppc_nvram_write,
> + .get_size = ppc_nvram_get_size,
> + .sync = ppc_nvram_sync,
> +};
Coming back to this after my comment on the m68k side, I notice that
there is now a double indirection through function pointers. Have you
considered completely removing the operations from ppc_md instead
by having multiple copies of nvram_ops?
With the current method, it does seem odd to have a single
per-architecture instance of the exported structure containing
function pointers. This doesn't give us the flexibility of having
multiple copies in the kernel the way that ppc_md does, but it adds
overhead compared to simply exporting the functions directly.
Arnd
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On 12/29/18 10:02 AM, Willy Tarreau wrote: > Ingo rightfully asked for a bit more documentation in the nolibc header, > so this patch adds some explanation about its purpose, how it's made, and > how to use it. > > Cc: Ingo Molnar > Cc: Paul E. McKenney > Cc: Randy Dunlap > Signed-off-by: Willy Tarreau Reviewed-by: Randy Dunlap Thanks. > --- > tools/testing/selftests/rcutorture/bin/nolibc.h | 90 > + > 1 file changed, 78 insertions(+), 12 deletions(-) > > diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h > b/tools/testing/selftests/rcutorture/bin/nolibc.h > index 985364c..6643ba9 100644 > --- a/tools/testing/selftests/rcutorture/bin/nolibc.h > +++ b/tools/testing/selftests/rcutorture/bin/nolibc.h > @@ -3,6 +3,84 @@ > * Copyright (C) 2017-2018 Willy Tarreau > */ > > +/* > + * This file is designed to be used as a libc alternative for minimal > programs > + * with very limited requirements. It consists of a small number of syscall > and > + * type definitions, and the minimal startup code needed to call main(). > + * All syscalls are declared as static functions so that they can be > optimized > + * away by the compiler when not used. > + * > + * Syscalls are split into 3 levels: > + * - the lower level is the arch-specific syscall() definition, consisting > in > + * assembly code in compound expressions. These are called my_syscall0() > to > + * my_syscall6() depending on the number of arguments. The MIPS > + * implementation is limited to 5 arguments. All input arguments are cast > + * to a long stored in a register. These expressions always return the > + * syscall's return value as a signed long value which is often either a > + * pointer or the negated errno value. > + * > + * - the second level is mostly architecture-independent. It is made of > + * static functions called sys_() which rely on my_syscallN() > + * depending on the syscall definition. These functions are responsible > + * for exposing the appropriate types for the syscall arguments (int, > + * pointers, etc) and for setting the appropriate return type (often > int). > + * A few of them are architecture-specific because the syscalls are not > all > + * mapped exactly the same among architectures. For example, some archs > do > + * not implement select() and need pselect6() instead, so the > sys_select() > + * function will have to abstract this. > + * > + * - the third level is the libc call definition. It exposes the lower raw > + * sys_() calls in a way that looks like what a libc usually does, > + * takes care of specific input values, and of setting errno upon error. > + * There can be minor variations compared to standard libc calls. For > + * example the open() call always takes 3 args here. > + * > + * The errno variable is declared static and unused. This way it can be > + * optimized away if not used. However this means that a program made of > + * multiple C files may observe different errno values (one per C file). For > + * the type of programs this project targets it usually is not a problem. The > + * resulting program may even be reduced by defining the NOLIBC_IGNORE_ERRNO > + * macro, in which case the errno value will never be assigned. > + * > + * Some stdint-like integer types are defined. These are valid on all > currently > + * supported architectures, because signs are enforced, ints are assumed to > be > + * 32 bits, longs the size of a pointer and long long 64 bits. If more > + * architectures have to be supported, this may need to be adapted. > + * > + * Some macro definitions like the O_* values passed to open(), and some > + * structures like the sys_stat struct depend on the architecture. > + * > + * The definitions start with the architecture-specific parts, which are > picked > + * based on what the compiler knows about the target architecture, and are > + * completed with the generic code. Since it is the compiler which sets the > + * target architecture, cross-compiling normally works out of the box without > + * having to specify anything. > + * > + * Finally some very common libc-level functions are provided. It is the case > + * for a few functions usually found in string.h, ctype.h, or stdlib.h. > Nothing > + * is currently provided regarding stdio emulation. > + * > + * The macro NOLIBC is always defined, so that it is possible for a program > to > + * check this macro to know if it is being built against and decide to > disable > + * some features or simply not to include some standard libc files. > + * > + * Ideally this file should be split in multiple files for easier long term > + * maintenance, but provided as a single file as it is now, it's quite > + * convenient to use. Maybe some variations involving a set of includes at > the > + * top could work. > + * > + * A simple static executable may be built this way : > + * $ gcc
Re: [PATCH] fanotify: allow freeze on suspend when waiting for response from userspace
On 12/29/18 3:04 PM, Orion Poplawski wrote:
On Thu 22-02-18 15:14:54, Kunal Shubham wrote:
>> On Fri 16-02-18 15:14:40, t.vi...@samsung.com wrote:
>> From: Vivek Trivedi
>> >> If fanotify userspace response server thread is frozen first,
>> it may fail to send response from userspace to kernel space listener.
>> In this scenario, fanotify response listener will never get response
>> from userepace and fail to suspend.
>> >> Use freeze-friendly wait API to handle this issue.
>> >> Same problem was reported here:
>> https://bbs.archlinux.org/viewtopic.php?id=232270
>> >> Freezing of tasks failed after 20.005 seconds
>> (1 tasks refusing to freeze, wq_busy=0)
>> >> Backtrace:
>> [] (__schedule) from [] (schedule+0x4c/0xa4)
>> [] (schedule) from []
(fanotify_handle_event+0x1c8/0x218)
>> [] (fanotify_handle_event) from []
(fsnotify+0x17c/0x38c)
>> [] (fsnotify) from []
(security_file_open+0x88/0x8c)
>> [] (security_file_open) from []
(do_dentry_open+0xc0/0x338)
>> [] (do_dentry_open) from [] (vfs_open+0x54/0x58)
>> [] (vfs_open) from []
(do_last.isra.10+0x45c/0xcf8)
>> [] (do_last.isra.10) from []
(path_openat+0x424/0x600)
>> [] (path_openat) from [] (do_filp_open+0x3c/0x98)
>> [] (do_filp_open) from []
(do_sys_open+0x120/0x1e4)
>> [] (do_sys_open) from [] (SyS_open+0x28/0x2c)
>> [] (SyS_open) from []
(__sys_trace_return+0x0/0x20)
>
> Yeah, good catch.
>
>> @@ -63,7 +64,9 @@ static int fanotify_get_response(struct
fsnotify_group *group,
>> >> pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>> >> - wait_event(group->fanotify_data.access_waitq,
event->response);
>> + while (!event->response)
>> + wait_event_freezable(group->fanotify_data.access_waitq,
>> + event->response);
>
> But if the process gets a signal while waiting, we will just
livelock the
> kernel in this loop as wait_event_freezable() will keep returning
> ERESTARTSYS. So you need to be a bit more clever here...
Hi Jack,
Thanks for the quick review.
To avoid livelock issue, is it fine to use below change? If agree, I
will send v2 patch.
@@ -63,7 +64,11 @@ static int fanotify_get_response(struct
fsnotify_group *group,
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ if
(wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response))
+ flush_signals(current);
+ }
Hum, I don't think this is correct either as this way if any signal was
delivered while waiting for fanotify response, we'd just lose it while
previously it has been properly handled. So what I think needs to be done
is that we just use wait_event_freezable() and propagate non-zero return
value (-ERESTARTSYS) up to the caller to handle the signal and restart
the
syscall as necessary.
Honza
--
Jan Kara
SUSE Labs, CR
Is there any progress here? This has become a real pain for us while
running BitDefender on EL7 laptops. I tried applying the following to
the EL7 kernel:
diff -up
linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
kernel-3.10.0-957.1.3.el7/linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
--- linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
2018-11-15 10:07:13.0 -0700
+++ linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
2018-12-28 15:44:26.452895337 -0700
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "fanotify.h"
@@ -64,7 +65,12 @@ static int fanotify_get_response(struct
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ ret =
wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response);
+ if (ret < 0)
+ return ret;
+ }
/* userspace responded, convert to something usable */
switch (event->response & ~FAN_AUDIT) {
but I get a kernel panic shortly after logging in to the system.
Here is the panic:
[ 324.774862] [ cut here ]
[ 324.774872] WARNING: CPU: 1 PID: 18685 at fs/notify/notification.c:84
fsnotify_destroy_event+0x6b/0x70
[ 324.774874] Modules linked in: cmac ccm xt_CHECKSUM iptable_mangle
ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat
nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack ipt_REJECT
nf_reject_ipv4 tun bridge stp llc devlink ebtable_filter ebtables
ip6table_filter ip6_tables iptable_filter bnep hid_multitouch iTCO_wdt
iTCO_vendor_support intel_wmi_thunderbolt dell_wmi arc4 iwlmvm
intel_pmc_core intel_powerclamp coretemp intel_rapl mac80211 kvm_intel
vfat fat dell_laptop kvm snd_hda_codec_hdmi
Re: [PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
On Sat, Dec 29, 2018 at 07:02:18PM +0100, Willy Tarreau wrote: > + * - the lower level is the arch-specific syscall() definition, consisting > in > + * assembly code in compound expressions. These are called my_syscall0() > to > + * my_syscall6() depending on the number of arguments. The MIPS > + * implementation is limited to 5 arguments. All input arguments are cast > + * to a long stored in a register. These expressions always return the > + * syscall's return value as a signed long value which is often either a > + * pointer or the negated errno value. > + * > + * - the second level is mostly architecture-independent. It is made of > + * static functions called sys_() which rely on my_syscallN() > + * depending on the syscall definition. These functions are responsible > + * for exposing the appropriate types for the syscall arguments (int, > + * pointers, etc) and for setting the appropriate return type (often > int). > + * A few of them are architecture-specific because the syscalls are not > all > + * mapped exactly the same among architectures. For example, some archs > do > + * not implement select() and need pselect6() instead, so the > sys_select() > + * function will have to abstract this. > + * > + * - the third level is the libc call definition. It exposes the lower raw > + * sys_() calls in a way that looks like what a libc usually does, > + * takes care of specific input values, and of setting errno upon error. > + * There can be minor variations compared to standard libc calls. For > + * example the open() call always takes 3 args here. Shouldn't these sentences begin with a capitalized "The" for consistency? > /* some archs (at least aarch64) don't expose the regular syscalls anymore by > * default, either because they have an "_at" replacement, or because there > are > * more modern alternatives. For now we'd rather still use them. Also here. Shouldn't this begin with a capitalized "Some"? -- Cheers, Joey Pabalinas signature.asc Description: PGP signature
Re: [PATCH 2/4] rcutorture/nolibc: fix some poor indentation and alignment
On Sat, Dec 29, 2018 at 07:02:17PM +0100, Willy Tarreau wrote:
> A few macros had their rightmost backslash misaligned, and the pollfd
> struct definition resisted the previous code reindent. Nothing else
> changed.
>
> Cc: Paul E. McKenney
> Signed-off-by: Willy Tarreau
Reviewed-by: Joey Pabalinas
> ---
> tools/testing/selftests/rcutorture/bin/nolibc.h | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
> b/tools/testing/selftests/rcutorture/bin/nolibc.h
> index 30bd27b..985364c 100644
> --- a/tools/testing/selftests/rcutorture/bin/nolibc.h
> +++ b/tools/testing/selftests/rcutorture/bin/nolibc.h
> @@ -81,9 +81,9 @@ typedef signed longtime_t;
>
> /* for poll() */
> struct pollfd {
> -int fd;
> -short int events;
> -short int revents;
> + int fd;
> + short int events;
> + short int revents;
> };
>
> /* for select() */
> @@ -239,7 +239,7 @@ struct stat {
> "syscall\n" \
> : "=a" (_ret) \
> : "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -255,7 +255,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -272,7 +272,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), "r"(_arg2), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> @@ -290,7 +290,7 @@ struct stat {
> : "=a" (_ret) \
> : "r"(_arg1), "r"(_arg2), "r"(_arg3), \
> "0"(_num) \
> - : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
> \
> + : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
> );\
> _ret; \
> })
> --
> 2.9.0
>
--
Cheers,
Joey Pabalinas
signature.asc
Description: PGP signature
[PATCH v6 1/2] signal: add pidfd_send_signal() syscall
The kill() syscall operates on process identifiers (pid). After a process has exited its pid can be reused by another process. If a caller sends a signal to a reused pid it will end up signaling the wrong process. This issue has often surfaced and there has been a push to address this problem [1]. This patch uses file descriptors (fd) from proc/ as stable handles on struct pid. Even if a pid is recycled the handle will not change. The fd can be used to send signals to the process it refers to. Thus, the new syscall pidfd_send_signal() is introduced to solve this problem. Instead of pids it operates on process fds (pidfd). /* prototype and argument /* long pidfd_send_signal(int pidfd, int sig, siginfo_t *info, unsigned int flags); In addition to the pidfd and signal argument it takes an additional siginfo_t and flags argument. If the siginfo_t argument is NULL then pidfd_send_signal() is equivalent to kill(, ). If it is not NULL pidfd_send_signal() is equivalent to rt_sigqueueinfo(). The flags argument is added to allow for future extensions of this syscall. It currently needs to be passed as 0. Failing to do so will cause EINVAL. /* pidfd_send_signal() replaces multiple pid-based syscalls */ The pidfd_send_signal() syscall currently takes on the job of rt_sigqueueinfo(2) and parts of the functionality of kill(2), Namely, when a positive pid is passed to kill(2). It will however be possible to also replace tgkill(2) and rt_tgsigqueueinfo(2) if this syscall is extended. /* sending signals to threads (tid) and process groups (pgid) */ Specifically, the pidfd_send_signal() syscall does currently not operate on process groups or threads. This is left for future extensions. In order to extend the syscall to allow sending signal to threads and process groups appropriately named flags (e.g. PIDFD_TYPE_PGID, and PIDFD_TYPE_TID) should be added. This implies that the flags argument will determine what is signaled and not the file descriptor itself. Put in other words, grouping in this api is a property of the flags argument not a property of the file descriptor (cf. [13]). Clarification for this has been requested by Eric (cf. [19]). When appropriate extensions through the flags argument are added then pidfd_send_signal() can additionally replace the part of kill(2) which operates on process groups as well as the tgkill(2) and rt_tgsigqueueinfo(2) syscalls. How such an extension could be implemented has been very roughly sketched in [14], [15], and [16]. However, this should not be taken as a commitment to a particular implementation. There might be better ways to do it. Right now this is intentionally left out to keep this patchset as simple as possible (cf. [4]). For example, if a pidfd for a tid from /proc//task/ is passed EOPNOTSUPP will be returned to give userspace a way to detect when I add support for signaling to threads (cf. [10]). /* naming */ The syscall had various names throughout iterations of this patchset: - procfd_signal() - procfd_send_signal() - taskfd_send_signal() In the last round of reviews it was pointed out that given that if the flags argument decides the scope of the signal instead of different types of fds it might make sense to either settle for "procfd_" or "pidfd_" as prefix. The community was willing to accept either (cf. [17] and [18]). Given that one developer expressed strong preference for the "pidfd_" prefix (cf. [13] and with other developers less opinionated about the name we should settle for "pidfd_" to avoid further bikeshedding. The "_send_signal" suffix was chosen to reflect the fact that the syscall takes on the job of multiple syscalls. It is therefore intentional that the name is not reminiscent of neither kill(2) nor rt_sigqueueinfo(2). Not the fomer because it might imply that pidfd_send_signal() is a replacement for kill(2), and not the latter because it is a hassle to remember the correct spelling - especially for non-native speakers - and because it is not descriptive enough of what the syscall actually does. The name "pidfd_send_signal" makes it very clear that its job is to send signals. /* zombies */ Zombies can be signaled just as any other process. No special error will be reported since a zombie state is an unreliable state (cf. [3]). However, this can be added as an extension through the @flags argument if the need ever arises. /* cross-namespace signals */ The patch currently enforces that the signaler and signalee either are in the same pid namespace or that the signaler's pid namespace is an ancestor of the signalee's pid namespace. This is done for the sake of simplicity and because it is unclear to what values certain members of struct siginfo_t would need to be set to (cf. [5], [6]). /* compat syscalls */ It became clear that we would like to avoid adding compat syscalls (cf. [7]). The compat syscall handling is now done in kernel/signal.c itself by adding __copy_siginfo_from_user_generic() which lets us avoid compat syscalls (cf. [8]).
[PATCH v6 2/2] selftests: add tests for pidfd_send_signal()
As suggested by Andrew Morton in [1] add selftests for the new
sys_pidfd_send_signal() syscall.
This tests whether we can send a signal to an existing process and whether
sending a signal to a process that has already exited fails with ESRCH.
[1]:
https://lore.kernel.org/lkml/20181228152012.dbf0508c2508138efc5f2...@linux-foundation.org/
Cc: Arnd Bergmann
Cc: "Eric W. Biederman"
Cc: Kees Cook
Cc: Serge Hallyn
Cc: Jann Horn
Cc: Andy Lutomirsky
Cc: Andrew Morton
Cc: Oleg Nesterov
Cc: Aleksa Sarai
Cc: Al Viro
Cc: Florian Weimer
Signed-off-by: Christian Brauner
---
/* Changelog */
v6:
- patch introduced
v5..v0:
- patch not present
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/pidfd/Makefile | 6 +
tools/testing/selftests/pidfd/pidfd_test.c | 130 +
3 files changed, 137 insertions(+)
create mode 100644 tools/testing/selftests/pidfd/Makefile
create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 24b9934fb269..63b0d8a0ebf7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -27,6 +27,7 @@ TARGETS += net
TARGETS += netfilter
TARGETS += networking/timestamping
TARGETS += nsfs
+TARGETS += pidfd
TARGETS += powerpc
TARGETS += proc
TARGETS += pstore
diff --git a/tools/testing/selftests/pidfd/Makefile
b/tools/testing/selftests/pidfd/Makefile
new file mode 100644
index ..deaf8073bc06
--- /dev/null
+++ b/tools/testing/selftests/pidfd/Makefile
@@ -0,0 +1,6 @@
+CFLAGS += -g -I../../../../usr/include/
+
+TEST_GEN_PROGS := pidfd_test
+
+include ../lib.mk
+
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c
b/tools/testing/selftests/pidfd/pidfd_test.c
new file mode 100644
index ..edcd59979b10
--- /dev/null
+++ b/tools/testing/selftests/pidfd/pidfd_test.c
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "../kselftest.h"
+
+static inline int sys_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
+ unsigned int flags)
+{
+ return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
+}
+
+static int signal_received;
+
+static void do_exit_success(int sig)
+{
+ signal_received = 1;
+}
+
+/*
+ * Straightforward test to see whether pidfd_send_signal() works is to send
+ * a signal to ourselves.
+ */
+static int test_pidfd_send_signal_simple_success(void)
+{
+ int pidfd, ret;
+ const char *test_name = "pidfd_send_signal send SIGUSR1";
+
+ pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC);
+ if (pidfd < 0)
+ ksft_exit_fail_msg(
+ "%s test: Failed to open process file descriptor\n",
+ test_name);
+
+ signal(SIGUSR1, do_exit_success);
+
+ ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0);
+ close(pidfd);
+ if (ret < 0)
+ ksft_exit_fail_msg("%s test: Failed to send signal\n",
+ test_name);
+
+ if (signal_received != 1)
+ ksft_exit_fail_msg("%s test: Failed to receive signal\n",
+ test_name);
+
+ signal_received = 0;
+ ksft_test_result_pass("%s test: Sent signal\n", test_name);
+ return 0;
+}
+
+static void wait_for_pid(pid_t pid)
+{
+ int status, ret;
+
+again:
+ ret = waitpid(pid, , 0);
+ if (ret == -1) {
+ if (errno == EINTR)
+ goto again;
+
+ return;
+ }
+
+ if (ret != pid)
+ goto again;
+}
+
+static int test_pidfd_send_signal_exited_fail(void)
+{
+ int pidfd, ret, saved_errno;
+ char buf[256];
+ pid_t pid;
+ const char *test_name = "pidfd_send_signal signal exited process";
+
+ pid = fork();
+ if (pid < 0)
+ ksft_exit_fail_msg("%s test: Failed to create new process\n",
+ test_name);
+
+ if (pid == 0)
+ _exit(EXIT_SUCCESS);
+
+ snprintf(buf, sizeof(buf), "/proc/%d", pid);
+
+ pidfd = open(buf, O_DIRECTORY | O_CLOEXEC);
+
+ wait_for_pid(pid);
+
+ if (pidfd < 0)
+ ksft_exit_fail_msg(
+ "%s test: Failed to open process file descriptor\n",
+ test_name);
+
+ ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0);
+ saved_errno = errno;
+ close(pidfd);
+ if (ret == 0)
+ ksft_exit_fail_msg(
+ "%s test: Managed to send signal to process even though
it should have failed\n",
+ test_name);
+
+ if (saved_errno != ESRCH)
+ ksft_exit_fail_msg(
+ "%s test: Expected to receive ESRCH as
Re: [PATCH v8 13/25] m68k: Dispatch nvram_ops calls to Atari or Mac functions
On Wed, Dec 26, 2018 at 1:43 AM Finn Thain wrote:
> +
> +static ssize_t m68k_nvram_get_size(void)
> +{
> + if (MACH_IS_ATARI)
> + return atari_nvram_get_size();
> + else if (MACH_IS_MAC)
> + return mac_pram_get_size();
> + return -ENODEV;
> +}
> +
> +/* Atari device drivers call .read (to get checksum validation) whereas
> + * Mac and PowerMac device drivers just use .read_byte.
> + */
> +const struct nvram_ops arch_nvram_ops = {
> +#ifdef CONFIG_MAC
> + .read_byte = m68k_nvram_read_byte,
> + .write_byte = m68k_nvram_write_byte,
> +#endif
> +#ifdef CONFIG_ATARI
> + .read = m68k_nvram_read,
> + .write = m68k_nvram_write,
> + .set_checksum = m68k_nvram_set_checksum,
> + .initialize = m68k_nvram_initialize,
> +#endif
> + .get_size = m68k_nvram_get_size,
> +};
> +EXPORT_SYMBOL(arch_nvram_ops);
Since the operations are almost entirely distinct, why not have two
separate 'nvram_ops' instances here that each refer to just
the set they actually need?
I was actually expecting one more patch here that would make the
arch_nvram_ops a pointer to one of multiple structures, which would
be easier to do with multiple copies, but I suppose there is no need
for that here (there might be on ppc, I have to look again).
Arnd
Re: [PATCH v8 18/25] powerpc: Implement nvram sync ioctl
> --- a/drivers/char/nvram.c
> +++ b/drivers/char/nvram.c
> @@ -48,6 +48,10 @@
> #include
> #include
>
> +#ifdef CONFIG_PPC
> +#include
> +#include
> +#endif
>
> static DEFINE_MUTEX(nvram_mutex);
> static DEFINE_SPINLOCK(nvram_state_lock);
> @@ -331,6 +335,37 @@ static long nvram_misc_ioctl(struct file *file, unsigned
> int cmd,
> long ret = -ENOTTY;
>
> switch (cmd) {
> +#ifdef CONFIG_PPC
> + case OBSOLETE_PMAC_NVRAM_GET_OFFSET:
> + pr_warn("nvram: Using obsolete PMAC_NVRAM_GET_OFFSET
> ioctl\n");
> + /* fall through */
> + case IOC_NVRAM_GET_OFFSET:
> + ret = -EINVAL;
> +#ifdef CONFIG_PPC_PMAC
I think it would make be nicer here to keep the ppc bits in arch/ppc,
and instead add a .ioctl() callback to nvram_ops.
> @@ -369,12 +405,14 @@ static int nvram_misc_open(struct inode *inode, struct
> file *file)
> return -EBUSY;
> }
>
> +#ifndef CONFIG_PPC
> /* Prevent multiple writers if the set_checksum ioctl is implemented.
> */
> if ((arch_nvram_ops.set_checksum != NULL) &&
> (file->f_mode & FMODE_WRITE) && (nvram_open_mode & NVRAM_WRITE)) {
> spin_unlock(_state_lock);
> return -EBUSY;
> }
> +#endif
>
> diff --git a/include/linux/nvram.h b/include/linux/nvram.h
> index b7bfaec60a43..24a57675dba1 100644
> --- a/include/linux/nvram.h
> +++ b/include/linux/nvram.h
> @@ -18,8 +18,12 @@ struct nvram_ops {
> unsigned char (*read_byte)(int);
> void(*write_byte)(unsigned char, int);
> ssize_t (*get_size)(void);
> +#ifdef CONFIG_PPC
> + long(*sync)(void);
> +#else
> long(*set_checksum)(void);
> long(*initialize)(void);
> +#endif
> };
Maybe just leave all entries visible here, and avoid the above #ifdef checks.
Arnd
Re: Re: [PATCH] fanotify: allow freeze on suspend when waiting for response from userspace
On Thu 22-02-18 15:14:54, Kunal Shubham wrote:
>> On Fri 16-02-18 15:14:40, t.vi...@samsung.com wrote:
>> From: Vivek Trivedi
>>
>> If fanotify userspace response server thread is frozen first,
>> it may fail to send response from userspace to kernel space listener.
>> In this scenario, fanotify response listener will never get response
>> from userepace and fail to suspend.
>>
>> Use freeze-friendly wait API to handle this issue.
>>
>> Same problem was reported here:
>> https://bbs.archlinux.org/viewtopic.php?id=232270
>>
>> Freezing of tasks failed after 20.005 seconds
>> (1 tasks refusing to freeze, wq_busy=0)
>>
>> Backtrace:
>> [] (__schedule) from [] (schedule+0x4c/0xa4)
>> [] (schedule) from [] (fanotify_handle_event+0x1c8/0x218)
>> [] (fanotify_handle_event) from [] (fsnotify+0x17c/0x38c)
>> [] (fsnotify) from [] (security_file_open+0x88/0x8c)
>> [] (security_file_open) from []
(do_dentry_open+0xc0/0x338)
>> [] (do_dentry_open) from [] (vfs_open+0x54/0x58)
>> [] (vfs_open) from [] (do_last.isra.10+0x45c/0xcf8)
>> [] (do_last.isra.10) from [] (path_openat+0x424/0x600)
>> [] (path_openat) from [] (do_filp_open+0x3c/0x98)
>> [] (do_filp_open) from [] (do_sys_open+0x120/0x1e4)
>> [] (do_sys_open) from [] (SyS_open+0x28/0x2c)
>> [] (SyS_open) from [] (__sys_trace_return+0x0/0x20)
>
> Yeah, good catch.
>
>> @@ -63,7 +64,9 @@ static int fanotify_get_response(struct fsnotify_group
*group,
>>
>> pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>>
>> - wait_event(group->fanotify_data.access_waitq, event->response);
>> + while (!event->response)
>> + wait_event_freezable(group->fanotify_data.access_waitq,
>> + event->response);
>
> But if the process gets a signal while waiting, we will just livelock the
> kernel in this loop as wait_event_freezable() will keep returning
> ERESTARTSYS. So you need to be a bit more clever here...
Hi Jack,
Thanks for the quick review.
To avoid livelock issue, is it fine to use below change?
If agree, I will send v2 patch.
@@ -63,7 +64,11 @@ static int fanotify_get_response(struct fsnotify_group
*group,
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ if (wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response))
+ flush_signals(current);
+ }
Hum, I don't think this is correct either as this way if any signal was
delivered while waiting for fanotify response, we'd just lose it while
previously it has been properly handled. So what I think needs to be done
is that we just use wait_event_freezable() and propagate non-zero return
value (-ERESTARTSYS) up to the caller to handle the signal and restart the
syscall as necessary.
Honza
--
Jan Kara
SUSE Labs, CR
Is there any progress here? This has become a real pain for us while
running BitDefender on EL7 laptops. I tried applying the following to
the EL7 kernel:
diff -up
linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
kernel-3.10.0-957.1.3.el7/linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
--- linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c.orig
2018-11-15 10:07:13.0 -0700
+++ linux-3.10.0-957.1.3.el7.x86_64/fs/notify/fanotify/fanotify.c
2018-12-28 15:44:26.452895337 -0700
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include "fanotify.h"
@@ -64,7 +65,12 @@ static int fanotify_get_response(struct
pr_debug("%s: group=%p event=%p\n", __func__, group, event);
- wait_event(group->fanotify_data.access_waitq, event->response);
+ while (!event->response) {
+ ret =
wait_event_freezable(group->fanotify_data.access_waitq,
+ event->response);
+ if (ret < 0)
+ return ret;
+ }
/* userspace responded, convert to something usable */
switch (event->response & ~FAN_AUDIT) {
but I get a kernel panic shortly after logging in to the system.
--
Orion Poplawski
Manager of NWRA Technical Systems 720-772-5637
NWRA, Boulder/CoRA Office FAX: 303-415-9702
3380 Mitchell Lane or...@nwra.com
Boulder, CO 80301 https://www.nwra.com/
Re: [GIT PULL 3/4] Kconfig updates for v4.21
The pull request you sent on Sat, 29 Dec 2018 01:04:29 +0900: > git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git > tags/kconfig-v4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/769e47094dcc0ddc8fe8e04c13565a71134ec1a2 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL 1/4] Kbuild updates for v4.21
The pull request you sent on Sat, 29 Dec 2018 00:58:38 +0900: > git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git > tags/kbuild-v4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/668c35f69cc750aaf07bd5fe7710a47e2aed6e43 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL 4/4] Kconfig file consolidation for v4.21
The pull request you sent on Sat, 29 Dec 2018 01:06:52 +0900: > git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git > tags/kconfig-v4.21-2 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/195303136f192d37b89e20a8d1d2670d0d825266 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [PATCH] Raise the minimum required gcc version to 4.6
On Sat, Dec 29, 2018 at 3:25 PM Geert Uytterhoeven wrote:
> On Fri, Aug 24, 2018 at 12:00 AM Joe Perches wrote:
> > On Thu, 2018-08-23 at 23:52 +0200, Geert Uytterhoeven wrote:
> ---
> build.log.linux-4.20.0-atari-07795-g835f16c9b68966ff-gcc-4.1.2-20061115-prerelease-Ubuntu-4.1.1-21
> +++
> build.log.linux-4.20.0-atari-07767-gc085b9fd60f52a62-gcc-7.3.0-27ubuntu1~18.04
>
> 20 warning regressions:
> + arch/m68k/atari/config.c: warning: ISO C90 forbids variable length
> array ‘switches’ [-Wvla]: => 151:2
Ah, so we still have some of these. The warning was only recently added.
> + arch/m68k/include/asm/cmpxchg.h: warning: value computed is not
> used [-Wunused-value]: => 79:22, 122:3, 137:3
IIRC this can be avoided using a ({ ... }) type expression.
> + arch/m68k/include/asm/raw_io.h: warning: cast to pointer from
> integer of different size [-Wint-to-pointer-cast]: => 20:19, 33:35,
> 26:31, 30:32
The I/O accessors are defined in an unusual way that defeats a lot
of the type checking we normally have. Generally speaking the
memory space operations (readl/ioread32/__raw_readl, ...) should
be inline functions taking a 'const volatile void __iomem *' argument
(non-const for writel), while the I/O space operations should take
an integer port number (16 or 32 bit, depending on how your ISA
or PCI buses work).
Doing that should avoid all the warnings you quote here, but may
introduce warnings about nonportable driver code.
> + arch/m68k/include/asm/string.h: warning: argument 2 null where
> non-null expected [-Wnonnull]: => 72:25
This might be a kernel bug.
> + arch/m68k/kernel/setup_mm.c: warning: #warning Are you building an
> allnoconfig kernel? [-Wcpp]: => 51:2
> + arch/m68k/kernel/setup_mm.c: warning: #warning No CPU/platform
> type selected, your kernel will not work! [-Wcpp]: => 50:2
> + arch/m68k/mvme147/config.c: warning: #warning check me! [-Wcpp]: => 150:2
> + arch/m68k/mvme16x/config.c: warning: #warning check me! [-Wcpp]: => 397:2
I've removed that kind of warning from other architectures.
> + arch/m68k/kernel/signal.c: warning: ISO C90 forbids variable
> length array ‘buf’ [-Wvla]: => 654:3
You can probably pick the maximum here.
> + drivers/i2c/i2c-core-base.c: warning: ‘ret’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]: => 235:5
This might come from the new CONFIG_NO_AUTO_INLINE.
> + drivers/input/joystick/analog.c: warning: #warning Precise timer
> not defined for this architecture. [-Wcpp]: => 172:2
Maybe add a Kconfig dependency on !M68K?
> + include/linux/dynamic_debug.h: warning: statement will never be
> executed [-Wswitch-unreachable]: => 115:19
No idea.
> + warning: unmet direct dependencies detected for NEED_MULTIPLE_NODES: =>
> N/A
> + warning: unmet direct dependencies detected for SND_SOC_QDSP6: => N/A
Not gcc warnings.
Arnd
Re: [PATCH] mm: Reuse only-pte-mapped KSM page in do_wp_page()
On Thu, 13 Dec 2018 18:29:08 +0300 Kirill Tkhai wrote:
> This patch adds an optimization for KSM pages almost
> in the same way, that we have for ordinary anonymous
> pages. If there is a write fault in a page, which is
> mapped to an only pte, and it is not related to swap
> cache; the page may be reused without copying its
> content.
>
> [Note, that we do not consider PageSwapCache() pages
> at least for now, since we don't want to complicate
> __get_ksm_page(), which has nice optimization based
> on this (for the migration case). Currenly it is
> spinning on PageSwapCache() pages, waiting for when
> they have unfreezed counters (i.e., for the migration
> finish). But we don't want to make it also spinning
> on swap cache pages, which we try to reuse, since
> there is not a very high probability to reuse them.
> So, for now we do not consider PageSwapCache() pages
> at all.]
>
> So, in reuse_ksm_page() we check for 1)PageSwapCache()
> and 2)page_stable_node(), to skip a page, which KSM
> is currently trying to link to stable tree. Then we
> do page_ref_freeze() to prohibit KSM to merge one more
> page into the page, we are reusing. After that, nobody
> can refer to the reusing page: KSM skips !PageSwapCache()
> pages with zero refcount; and the protection against
> of all other participants is the same as for reused
> ordinary anon pages pte lock, page lock and mmap_sem.
>
> ...
>
> +bool reuse_ksm_page(struct page *page,
> + struct vm_area_struct *vma,
> + unsigned long address)
> +{
> + VM_BUG_ON_PAGE(is_zero_pfn(page_to_pfn(page)), page);
> + VM_BUG_ON_PAGE(!page_mapped(page), page);
> + VM_BUG_ON_PAGE(!PageLocked(page), page);
> +
> + if (PageSwapCache(page) || !page_stable_node(page))
> + return false;
> + /* Prohibit parallel get_ksm_page() */
> + if (!page_ref_freeze(page, 1))
> + return false;
> +
> + page_move_anon_rmap(page, vma);
> + page->index = linear_page_index(vma, address);
> + page_ref_unfreeze(page, 1);
> +
> + return true;
> +}
Can we avoid those BUG_ON()s?
Something like this:
--- a/mm/ksm.c~mm-reuse-only-pte-mapped-ksm-page-in-do_wp_page-fix
+++ a/mm/ksm.c
@@ -2649,9 +2649,14 @@ bool reuse_ksm_page(struct page *page,
struct vm_area_struct *vma,
unsigned long address)
{
- VM_BUG_ON_PAGE(is_zero_pfn(page_to_pfn(page)), page);
- VM_BUG_ON_PAGE(!page_mapped(page), page);
- VM_BUG_ON_PAGE(!PageLocked(page), page);
+#ifdef CONFIG_DEBUG_VM
+ if (WARN_ON(is_zero_pfn(page_to_pfn(page))) ||
+ WARN_ON(!page_mapped(page)) ||
+ WARN_ON(!PageLocked(page))) {
+ dump_page(page, "reuse_ksm_page");
+ return false;
+ }
+#endif
if (PageSwapCache(page) || !page_stable_node(page))
return false;
We don't have a VM_WARN_ON_PAGE() and we can't provide one because the
VM_foo() macros don't return a value. It's irritating and I keep
forgetting why we ended up doing them this way.
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
On Sat, Dec 29, 2018 at 3:51 AM Michael Schmitz wrote: > > Hi Finn, > > Am 29.12.2018 um 15:34 schrieb Finn Thain: > > On Sat, 29 Dec 2018, Michael Schmitz wrote: > > > >> > >> IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to > >> suggest. > >> > >> Or (really going out on a limb here): > >> > >> IS_BUILTIN(CONFIG_NVRAM) || > >> ( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) ) > >> > >> Not that I'd advocate that, for this series. > >> > > > > Well, you are a maintainer for atari_scsi.c. > > > > Are you saying that you want IS_BUILTIN(CONFIG_NVRAM) used here instead of > > ifdef? > > No, just pointing out that there would be a way to avoid the ifdef > without messing up driver behaviour. I'm fine with the ifdef - not least > because it clearly eliminates code that would be unreachable. > > (On second thought - I don't want to speculate whether there's weird > compiler options that could result in the nvram_check_checksum and > nvram_read_bytes symbols to still be referenced in the final link, even > though IS_BUILTIN(CONFIG_NVRAM) always evaluates to false. Best leave > this as-is.) As far as I know, it's totally reliable with the supported compilers (gcc-4.6+). In the older compilers (e.g. 4.1), there was a corner case, where it could have failed to eliminate a function that was only referenced through a pointer from a discarded variable, but a plain IS_ENABLED() check like the one here was still ok, and lots of code relies on that. Other than that, I agree either way is totally fine here, so no objections to using the #ifdef. Arnd
[PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path
If gpiod_get_optional() fails in tw9910_power_on(), clk is left undisabled.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/media/i2c/tw9910.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index a54548cc4285..109770d678d2 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -610,6 +610,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
GPIOD_OUT_LOW);
if (IS_ERR(priv->rstb_gpio)) {
dev_info(>dev, "Unable to get GPIO \"rstb\"");
+ clk_disable_unprepare(priv->clk);
return PTR_ERR(priv->rstb_gpio);
}
--
2.7.4
Re: [PATCH] percpu: plumb gfp flag to pcpu_get_pages
Hi Dennis, On Sat, Dec 29, 2018 at 1:26 PM Dennis Zhou wrote: > > Hi Andrew, > > On Sat, Dec 29, 2018 at 01:03:52PM -0800, Andrew Morton wrote: > > On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt wrote: > > > > > __alloc_percpu_gfp() can be called from atomic context, so, make > > > pcpu_get_pages use the gfp provided to the higher layer. > > > > Does this fix any user-visible issues? > > Sorry for not getting to this earlier. I'm currently traveling. I > respoeded on the patch itself. Do you mind unqueuing? I explain in more > detail on the patch, but __alloc_percpu_gfp() will never call > pcpu_get_pages() when called as not GFP_KERNEL. > Thanks for the explanation. Andrew, please ignore/drop this patch. thanks, Shakeel
Re: [PATCH] percpu: plumb gfp flag to pcpu_get_pages
Hi Andrew, On Sat, Dec 29, 2018 at 01:03:52PM -0800, Andrew Morton wrote: > On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt wrote: > > > __alloc_percpu_gfp() can be called from atomic context, so, make > > pcpu_get_pages use the gfp provided to the higher layer. > > Does this fix any user-visible issues? Sorry for not getting to this earlier. I'm currently traveling. I respoeded on the patch itself. Do you mind unqueuing? I explain in more detail on the patch, but __alloc_percpu_gfp() will never call pcpu_get_pages() when called as not GFP_KERNEL. Thanks, Dennis
Re: [PATCH] percpu: plumb gfp flag to pcpu_get_pages
Hi Shakeel,
On Fri, Dec 28, 2018 at 05:31:47PM -0800, Shakeel Butt wrote:
> __alloc_percpu_gfp() can be called from atomic context, so, make
> pcpu_get_pages use the gfp provided to the higher layer.
>
> Signed-off-by: Shakeel Butt
> ---
> mm/percpu-vm.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/mm/percpu-vm.c b/mm/percpu-vm.c
> index d8078de912de..4f42c4c5c902 100644
> --- a/mm/percpu-vm.c
> +++ b/mm/percpu-vm.c
> @@ -21,6 +21,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk
> *chunk,
>
> /**
> * pcpu_get_pages - get temp pages array
> + * @gfp: allocation flags passed to the underlying allocator
> *
> * Returns pointer to array of pointers to struct page which can be indexed
> * with pcpu_page_idx(). Note that there is only one array and accesses
> @@ -29,7 +30,7 @@ static struct page *pcpu_chunk_page(struct pcpu_chunk
> *chunk,
> * RETURNS:
> * Pointer to temp pages array on success.
> */
> -static struct page **pcpu_get_pages(void)
> +static struct page **pcpu_get_pages(gfp_t gfp)
> {
> static struct page **pages;
> size_t pages_size = pcpu_nr_units * pcpu_unit_pages * sizeof(pages[0]);
> @@ -37,7 +38,7 @@ static struct page **pcpu_get_pages(void)
> lockdep_assert_held(_alloc_mutex);
>
> if (!pages)
> - pages = pcpu_mem_zalloc(pages_size, GFP_KERNEL);
> + pages = pcpu_mem_zalloc(pages_size, gfp);
> return pages;
> }
>
> @@ -278,7 +279,7 @@ static int pcpu_populate_chunk(struct pcpu_chunk *chunk,
> {
> struct page **pages;
>
> - pages = pcpu_get_pages();
> + pages = pcpu_get_pages(gfp);
> if (!pages)
> return -ENOMEM;
>
> @@ -316,7 +317,7 @@ static void pcpu_depopulate_chunk(struct pcpu_chunk
> *chunk,
>* successful population attempt so the temp pages array must
>* be available now.
>*/
> - pages = pcpu_get_pages();
> + pages = pcpu_get_pages(GFP_KERNEL);
> BUG_ON(!pages);
>
> /* unmap and free */
> --
> 2.20.1.415.g653613c723-goog
>
Sorry, I'm travelling today and was hoping to respond to this later
tonight.
So percpu memory is a little different as it's an intermediary. When you
call __alloc_percpu_gfp() and it does not contain GFP_KERNEL, it is
considered atomic. So, we only service requests out of already
populated memory. pcpu_get_pages() is only called when we need to
populate/depopulate a chunk and will not be called if we need an atomic
allocation. Also, in all but the first case, it won't make an allocation
as pages is a static variable.
Furthermore, percpu only plumbs through certain gfp as not all make
sense [1].
[1] https://lore.kernel.org/lkml/cover.1518668149.git.dennissz...@gmail.com/
Thanks,
Dennis
: I need your urgent respons.
Dear Friend,
I know that this mail will come to you as a surprise as we have never met
before,I need your Urgent assistance in transferring the sum of $18 mllion US
Dollars immediately to your Private account.The money has been here in our Bank
lying dormant for years now without anybody coming for the claim of it.Waiting
to hear from you.
The information required should include the following-:
Your full names
Your address
Telephone number
Your private email
Occupation
Age
Please reply me through my email address. Email: { cobosulem...@gmail.com }
this is to enable my further discussion with you in confidence.
Best regards and wishes to you.
Mr.Cobo Suleman
Re: [PATCH] percpu: plumb gfp flag to pcpu_get_pages
On Fri, 28 Dec 2018 17:31:47 -0800 Shakeel Butt wrote: > __alloc_percpu_gfp() can be called from atomic context, so, make > pcpu_get_pages use the gfp provided to the higher layer. Does this fix any user-visible issues?
Re: [GIT PULL 2/4] Kbuild updates for v4.21 part2
On Fri, Dec 28, 2018 at 8:00 AM Masahiro Yamada wrote: > > Introduce a new option CONFIG_NO_AUTO_INLINE as well. With this option, > only functions explicitly marked with "inline" will be inlined. This > will allow the function tracer to trace more functions. Ugh. This causes new and bogus warnings, because gcc doesn't see some of the checks. For example, the x86 emulate_vsyscall() function now warns that warning: ‘syscall_nr’ may be used uninitialized in this function because addr_to_vsyscall_nr() isn't inlined any more, so gcc doesn't see that the return value is always smaller than three (and then it doesn't see that we handle all the cases in the switch statement later). So honestly, these "debugging improvements" have a rather nasty side effect. I'm not at all convinced that we should do this. It is *not* worth causing extra development pain with a totally pointless option that changes code generation radically like this. It's going to cause lots of extra churn. This branch already added a couple of extra inline markers just to make code work reasonably. How many tens (or hundreds) got missed just because the build still works, but the lack of inlining means that we generate completely garbage code? I'm going to skip this pull request. We have basically always required a certain level of competence from the compiler, and this basically says that the compiler can do stupid things and we'd have to fix those stupidities by hand. Linus
Re: [PATCH -mmotm] efi: drop kmemleak_ignore() for page allocator
On 12/29/18 4:17 AM, Ard Biesheuvel wrote: > On Fri, 28 Dec 2018 at 04:04, Andrew Morton wrote: >> >> On Wed, 26 Dec 2018 16:31:59 +0100 Ard Biesheuvel >> wrote: >> >>> Please stop sending EFI patches if you can't be bothered to >>> test/reproduce against the EFI tree. >> >> um, sorry, but that's a bit strong. Finding (let alone fixing) a bug >> in EFI is a great contribution (thanks!) and the EFI maintainers are >> perfectly capable of reviewing and testing the proposed fix. Or of >> fixing the bug by other means. >> > > Qian did spot some issues recently, which was really helpful. But I > really think that reporting all issues you find against the -mmotm > tree because that happens to be your preferred tree for development is > not the correct approach. > >> Let's not beat people up for helping us in a less-than-perfect way, no? > > Fair enough. But asking people to ensure that an issue they found > actually exists on the subsystem tree in question is not that much to > ask, is it? It is not too much to ask to test on EFI subsystem tree only for this patch, but if every maintainer asked for the same thing to test each subsystem tree after found a bug even a trivial one in -mmotm or linux-next, it then become an issue. There are people genuinely interested in the kernel in general rather than focus on a few subsystems (yet). There are many subsystem git trees out there. It at least needs to figure out which branch to test and adjust the config file accordingly. Those subsystem trees usually are not well-documented like linux-next or -mmotm trees. Then, they may need to deal with the subsystem tree-specific issues. Those people may just better switch to use mainline instead where they don't need to bother testing the subsystem tree for every single patch. However, that will cause delay in fixing those issues because mainline is usually a bit lag behind the development.
Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
Am 29.12.18 um 20:25 schrieb Andreas Färber:
> Am 12.10.18 um 18:26 schrieb Ben Whitten:
>> +static int sx125x_register_clock_provider(struct sx125x_priv *priv)
>> +{
>> +struct device *dev = priv->dev;
>> +struct clk_init_data init;
>> +const char *parent;
>> +int ret;
>> +
>> +/* Disable CLKOUT */
>> +ret = sx125x_field_write(priv, F_CLK_OUT, 0);
>> +if (ret) {
>> +dev_err(dev, "unable to disable clkout\n");
>> +return ret;
>> +}
>> +
>> +/* Register clock provider if expected in DTB */
>> +if (!of_find_property(dev->of_node, "#clock-cells", NULL))
>> +return 0;
>> +
>> +dev_info(dev, "registering clkout\n");
>> +
>> +parent = of_clk_get_parent_name(dev->of_node, 0);
>> +if (!parent) {
>> +dev_err(dev, "Unable to find parent clk\n");
>> +return -ENODEV;
>
> I got stuck here testing:
>
> [233875.731268] sx1301 spi0.0: SX1301 module probed
> [233876.520801] sx1301 spi1.0: SX1301 module probed
> [233876.543460] sx125x_con spi0.0-b: SX125x version: 21
> [233876.550866] sx125x_con spi0.0-b: registering clkout
> [233876.555852] sx125x_con spi0.0-b: Unable to find parent clk
> [233876.561491] sx125x_con spi0.0-b: failed to register clkout provider: -19
> [233876.569914] sx125x_con spi0.0-a: SX125x version: 21
> [233876.582915] sx125x_con spi0.0-a: SX125x module probed
> [233876.589100] sx125x_con spi1.0-b: SX125x version: 21
> [233876.595981] sx125x_con spi1.0-b: registering clkout
> [233876.600986] sx125x_con spi1.0-b: Unable to find parent clk
> [233876.606557] sx125x_con spi1.0-b: failed to register clkout provider: -19
> [233876.614559] sx125x_con spi1.0-a: SX125x version: 21
> [233876.625047] sx125x_con spi1.0-a: SX125x module probed
>
> Your DT example above does not use any parent clock for the radios. It
> seems adding any clocks = <> reference helps resolve that error.
>
> I don't spot any code dealing with enabling that parent either. With a
> fixed-clock we appear to get around that, except for reference counting:
>
This was before `sudo ip link set lora1 up`...
> # cat /sys/kernel/debug/clk/clk_summary
> [...]
> rak831-clk32m0003200
>0 0 5
> rak831_clk32m 0003200
>0 0 5
> rg186-clk32m 0003200
>0 0 5
> rg186_clk32m 0003200
>0 0 5
Afterwards things seemed okay, but I didn't capture clk_summary again.
>
> But it might just as well be a gpio-gate-clock or some PMIC providing
> it, needing prepare+enable ops.
>
>> +}
>> +
>> +init.ops = _clkout_ops;
>> +init.flags = CLK_IS_BASIC;
>
> I don't think that's really true.
>
>> +init.parent_names =
>> +init.num_parents = 1;
>> +priv->clkout_hw.init =
>> +
>> +of_property_read_string_index(dev->of_node, "clock-output-names", 0,
>> +);
>
> Error handling for this was in your style cleanup patch. I'd prefer to
> squash that hunk here.
>
> However, clock-output-names is documented as an optional property, so we
> shouldn't rely on it here, please.
>
>> +
>> +priv->clkout = devm_clk_register(dev, >clkout_hw);
>> +if (IS_ERR(priv->clkout)) {
>> +dev_err(dev, "failed to register clkout\n");
>
> Would be nice to output the error code, but then again the caller does.
>
>> +return PTR_ERR(priv->clkout);
>> +}
>> +ret = of_clk_add_hw_provider(dev->of_node, of_clk_hw_simple_get,
>> +>clkout_hw);
>
>
> Don't we need to unregister the provider on remove? Using the devm_
> variant would seem to handle that for us.
Possibly related, `sudo ip link set lora2 up` froze my system, with both
lora1 and lora2 being sx1301. (I should script bringing all of them up!)
Investigating...
And thanks go to the multiple vendors who donated SX130x hardware for
test coverage, helping catch non-standard bugs like these early!
Regards,
Andreas
>> +return ret;
>> +}
[snip]
[ 473.605058] sx1301 spi0.0: SX1301 module probed
[ 474.394760] sx1301 spi1.0: SX1301 module probed
[ 485.637333] sx125x_con spi0.0-b: SX125x version: 21
[ 485.645801] sx125x_con spi0.0-b: registering clkout
[ 485.656684] sx125x_con spi0.0-b: SX125x module probed
[ 485.663789] sx125x_con spi0.0-a: SX125x version: 21
[ 485.677570] sx125x_con spi0.0-a: SX125x module probed
[ 485.685713] sx125x_con spi1.0-b: SX125x version: 21
[ 485.692210] sx125x_con spi1.0-b: registering clkout
[ 485.701712] sx125x_con spi1.0-b: SX125x module probed
[ 485.708067] sx125x_con spi1.0-a: SX125x version: 21
[ 485.718707] sx125x_con spi1.0-a: SX125x module probed
[...]
[ 4603.171814] sx125x_con spi0.0-b: enabling clkout
[ 4603.697489] sx1301 spi0.0: AGC calibration firmware version 2
[ 4603.703782] sx1301 spi0.0: starting
Re: [PATCH] tools headers: move the nolibc header from rcutorture to tools/include/nolibc/
On Sat, Dec 29, 2018 at 11:55:18AM -0800, Paul E. McKenney wrote: > On Sat, Dec 29, 2018 at 07:30:47PM +0100, Willy Tarreau wrote: > > On Sat, Dec 29, 2018 at 10:25:08AM -0800, Paul E. McKenney wrote: > > > On Sat, Dec 29, 2018 at 07:04:53PM +0100, Willy Tarreau wrote: > > > > As suggested by Ingo, this header file might benefit other tools than > > > > just rcutorture. For now it's quite limited, but is easy to extend, so > > > > exposing it into tools/include/nolibc/ will make it much easier to > > > > adopt by other tools. > > > > > > > > The mkinitrd.sh script in rcutorture was updated to use this new > > > > location. > > > > > > > > Cc: Ingo Molnar > > > > Cc: Arnaldo Carvalho de Melo > > > > Cc: Paul E. McKenney > > > > Signed-off-by: Willy Tarreau > > > > > > Thank you, Willy! > > > > You're welcome! > > > > > I have queued all four of these. > > > > Thanks! > > > > > Should there > > > be a MAINTAINERS file entry for the new include/nolibc home for this > > > library code? > > > > Good idea, I didn't think about it. Yes, I can add one and will > > send another patch. In case that helps I've created a repo at > > /pub/scm/linux/kernel/git/wtarreau/nolibc.git > > And I have queued that one as well, thank you! > > I am happy to curate nolibc patches indefinitely, but should some other > pathway to mainline become better for you, just let me know. Oh perfect, thank you. I'm always having difficulties with processes, so having a known working path to mainline is indeed a much appreciated help! Thanks, Willy
Re: [PATCH] tools headers: move the nolibc header from rcutorture to tools/include/nolibc/
On Sat, Dec 29, 2018 at 07:30:47PM +0100, Willy Tarreau wrote: > On Sat, Dec 29, 2018 at 10:25:08AM -0800, Paul E. McKenney wrote: > > On Sat, Dec 29, 2018 at 07:04:53PM +0100, Willy Tarreau wrote: > > > As suggested by Ingo, this header file might benefit other tools than > > > just rcutorture. For now it's quite limited, but is easy to extend, so > > > exposing it into tools/include/nolibc/ will make it much easier to > > > adopt by other tools. > > > > > > The mkinitrd.sh script in rcutorture was updated to use this new location. > > > > > > Cc: Ingo Molnar > > > Cc: Arnaldo Carvalho de Melo > > > Cc: Paul E. McKenney > > > Signed-off-by: Willy Tarreau > > > > Thank you, Willy! > > You're welcome! > > > I have queued all four of these. > > Thanks! > > > Should there > > be a MAINTAINERS file entry for the new include/nolibc home for this > > library code? > > Good idea, I didn't think about it. Yes, I can add one and will > send another patch. In case that helps I've created a repo at > /pub/scm/linux/kernel/git/wtarreau/nolibc.git And I have queued that one as well, thank you! I am happy to curate nolibc patches indefinitely, but should some other pathway to mainline become better for you, just let me know. Thanx, Paul
Re: [GIT PULL] Documentation for 5.0
The pull request you sent on Fri, 28 Dec 2018 09:06:22 -0700: > git://git.lwn.net/linux.git tags/docs-5.0 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/3868772b99e3146d02cf47e739d79022eba1d77c Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT UPDATED PULL] cgroup changes for v4.21-rc1
The pull request you sent on Fri, 28 Dec 2018 10:41:17 -0800: > git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/6f9d71c9c759b1e7d31189a4de228983192c7dc7 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] cgroup changes for v4.21-rc1
The pull request you sent on Thu, 27 Dec 2018 18:16:05 -0800: > git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e250d91d65750a0c0c62483ac4f9f357e7317617 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
[GIT PULL] dax fix for 4.21
Hi Linus, please pull from: git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fix-4.21 ...to receive a single fix for an issue you identified on the last dax fix pull request. While I feel a bit silly sending a single-commit pull-request there is nothing else queued up for dax this cycle. This change has shipped in -next for multiple releases. --- The following changes since commit 7566ec393f4161572ba6f11ad5171fd5d59b0fbd: Linux 4.20-rc7 (2018-12-16 15:46:55 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm tags/dax-fix-4.21 for you to fetch changes up to d8a706414af4827fc0b4b1c0c631c607351938b9: dax: Use non-exclusive wait in wait_entry_unlocked() (2018-12-21 11:35:53 -0800) dax fix 4.21 * Clean up unnecessary usage of prepare_to_wait_exclusive() Dan Williams (1): dax: Use non-exclusive wait in wait_entry_unlocked() fs/dax.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-)
Re: [PATCH 1/1] x86/gart/kcore: Exclude GART aperture from kcore
Hi Kairui,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20 next-20181224]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Kairui-Song/x86-gart-kcore-Exclude-GART-aperture-from-kcore/20181221-132322
config: x86_64-randconfig-k2-12281339 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
arch/x86/kernel/aperture_64.o: In function `exclude_from_vmcore':
>> arch/x86/kernel/aperture_64.c:81: undefined reference to
>> `register_mem_pfn_is_ram'
vim +81 arch/x86/kernel/aperture_64.c
75
76 static void exclude_from_vmcore(u64 aper_base, u32 aper_order)
77 {
78 aperture_pfn_start = aper_base >> PAGE_SHIFT;
79 aperture_page_count = (32 * 1024 * 1024) << aper_order >>
PAGE_SHIFT;
80 WARN_ON(register_oldmem_pfn_is_ram(_mem_pfn_is_ram));
> 81 WARN_ON(register_mem_pfn_is_ram(_mem_pfn_is_ram));
82 }
83 #else
84 static void exclude_from_vmcore(u64 aper_base, u32 aper_order)
85 {
86 }
87 #endif
88
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: Re: [PATCH] netfilter: account ebt_table_info to kmemcg
Hi Kirill, On Sat, Dec 29, 2018 at 1:52 AM Kirill Tkhai wrote: > > Hi, Michal! > > On 29.12.2018 10:33, Michal Hocko wrote: > > On Fri 28-12-18 17:55:24, Shakeel Butt wrote: > >> The [ip,ip6,arp]_tables use x_tables_info internally and the underlying > >> memory is already accounted to kmemcg. Do the same for ebtables. The > >> syzbot, by using setsockopt(EBT_SO_SET_ENTRIES), was able to OOM the > >> whole system from a restricted memcg, a potential DoS. > > > > What is the lifetime of these objects? Are they bound to any process? > > These are list of ebtables rules, which may be displayed with $ebtables-save > command. > In case of we do not account them, a low priority container may eat all the > memory > and OOM killer in berserk mode will kill all the processes on machine. They > are not bound > to any process, but they are bound to network namespace. > > OOM killer does not analyze such the memory cgroup-related allocations, since > it > is task-aware only. Maybe we should do it namespace-aware too... This is a good idea. I am already brainstorming on a somewhat similar idea to make shmem/tmpfs files oom-killable. I will share once I have something more concrete and will think on namespace angle too. thanks, Shakeel
Re: [PATCH] netfilter: account ebt_table_info to kmemcg
On Sat, Dec 29, 2018 at 2:06 AM Michal Hocko wrote: > > On Sat 29-12-18 10:52:15, Florian Westphal wrote: > > Michal Hocko wrote: > > > On Fri 28-12-18 17:55:24, Shakeel Butt wrote: > > > > The [ip,ip6,arp]_tables use x_tables_info internally and the underlying > > > > memory is already accounted to kmemcg. Do the same for ebtables. The > > > > syzbot, by using setsockopt(EBT_SO_SET_ENTRIES), was able to OOM the > > > > whole system from a restricted memcg, a potential DoS. > > > > > > What is the lifetime of these objects? Are they bound to any process? > > > > No, they are not. > > They are free'd only when userspace requests it or the netns is > > destroyed. > > Then this is problematic, because the oom killer is not able to > guarantee the hard limit and so the excessive memory consumption cannot > be really contained. As a result the memcg will be basically useless > until somebody tears down the charged objects by other means. The memcg > oom killer will surely kill all the existing tasks in the cgroup and > this could somehow reduce the problem. Maybe this is sufficient for > some usecases but that should be properly analyzed and described in the > changelog. > Can you explain why you think the memcg hard limit will not be enforced? From what I understand, the memcg oom-killer will kill the allocating processes as you have mentioned. We do force charging for very limited conditions but here the memcg oom-killer will take care of Anyways, the kernel is already charging the memory for [ip,ip6,arp]_tables and this patch adds the charging for ebtables. Without this patch, as Kirill has described and shown by syzbot, a low priority memcg can force system OOM. Shakeel
Re: [PATCH v3 lora-next 5/5] net: lora: sx125x sx1301: allow radio to register as a clk provider
Hi Ben,
+ linux-lpwan, linux-clk, devicetree
Am 12.10.18 um 18:26 schrieb Ben Whitten:
> From: Ben Whitten
>
> The 32M is run from the radio, before we just enabled it based on
> the radio number but now we can use the clk framework to request the
> clk is started when we need it.
>
> The 32M clock produced from the radio is really a gated version of
> tcxo which is a fixed clock provided by hardware, and isn't captured
> in this patch.
>
> The sx1301 brings the clock up prior to calibration once the radios
> have probed themselves.
>
> A sample dts showing the clk link:
> sx1301: sx1301@0 {
Nit: Node names should not duplicate model from compatible or label.
I've been using "lora" for both concentrator and radios.
> ...
> clocks = < 0>;
Since you use #clock-cells of zero below, you are specifying two clocks
here, surely unintentional.
> clock-names = "clk32m";
>
> radio-spi {
> radio0: radio-a@0 {
-a/-b in node names duplicates the unit address. I've been using "lora",
but we might also go for just "radio" or "transceiver"?
> ...
> };
>
> radio1: radio-b@1 {
Should add "..." here, too, for clarity.
> #clock-cells = <0>;
> clock-output-names = "clk32m";
Personally I'd reorder those two lines to have #foo-cells last.
But more importantly, in my testing this is lacking, e.g.,
clocks = <>;
and an appropriate fixed-clock node in the root node. See inline.
> };
> };
> };
This issue is making me think we should start properly documenting our
dt-bindings in a preceding patch, even if just for my linux-lora staging
tree. Be it in the old .txt format or Rob's newly proposed YAML.
The more cooks are working on SX130x, the better we need to explain what
changes people need to make to their DT to keep things working - if I
stumble as original author, then new testers are even more likely to!
>
> Signed-off-by: Ben Whitten
> ---
> drivers/net/lora/sx125x.c | 112
> ++
> drivers/net/lora/sx1301.c | 13 ++
> drivers/net/lora/sx1301.h | 2 +
> 3 files changed, 119 insertions(+), 8 deletions(-)
In general I'd appreciate if you would prepare, e.g., the clock output
in a preceding sx125x-only patch. Exposing (and consuming) clocks in the
radio driver should be independent of consuming them in the concentrator
driver. That'll make things easier to review for us and also helps avoid
the unusual "sx125x sx1301" in the subject, here and elsewhere.
>
> diff --git a/drivers/net/lora/sx125x.c b/drivers/net/lora/sx125x.c
> index 36b61b1..b7ca782 100644
> --- a/drivers/net/lora/sx125x.c
> +++ b/drivers/net/lora/sx125x.c
> @@ -9,6 +9,8 @@
> * Copyright (c) 2013 Semtech-Cycleo
> */
>
> +#include
> +#include
> #include
> #include
> #include
> @@ -42,10 +44,16 @@ static const struct reg_field sx125x_regmap_fields[] = {
> };
>
> struct sx125x_priv {
> + struct clk *clkout;
> + struct clk_hw clkout_hw;
The 0-day bots have reported this to break on sparc64, as you've seen.
We'll need to squash a Kconfig or .c based solution here.
> +
> + struct device *dev;
> struct regmap *regmap;
> struct regmap_field
> *regmap_fields[ARRAY_SIZE(sx125x_regmap_fields)];
> };
>
> +#define to_clkout(_hw) container_of(_hw, struct sx125x_priv, clkout_hw)
> +
> static struct regmap_config __maybe_unused sx125x_regmap_config = {
> .reg_bits = 8,
> .val_bits = 8,
> @@ -64,6 +72,96 @@ static int sx125x_field_write(struct sx125x_priv *priv,
> return regmap_field_write(priv->regmap_fields[field_id], val);
> }
>
> +static int sx125x_field_read(struct sx125x_priv *priv,
> + enum sx125x_fields field_id, unsigned int *val)
> +{
> + return regmap_field_read(priv->regmap_fields[field_id], val);
> +}
Nit: Given how trivial this is, we might make it static inline?
> +
> +static int sx125x_clkout_enable(struct clk_hw *hw)
> +{
> + struct sx125x_priv *priv = to_clkout(hw);
> +
> + dev_info(priv->dev, "enabling clkout\n");
> + return sx125x_field_write(priv, F_CLK_OUT, 1);
> +}
> +
> +static void sx125x_clkout_disable(struct clk_hw *hw)
> +{
> + struct sx125x_priv *priv = to_clkout(hw);
> + int ret;
> +
> + dev_info(priv->dev, "disabling clkout\n");
> + ret = sx125x_field_write(priv, F_CLK_OUT, 0);
> + if (ret)
> + dev_err(priv->dev, "error disabling clkout\n");
> +}
> +
> +static int sx125x_clkout_is_enabled(struct clk_hw *hw)
> +{
> + struct sx125x_priv *priv = to_clkout(hw);
> + unsigned int enabled;
> + int ret;
> +
> + ret = sx125x_field_read(priv, F_CLK_OUT, );
> + if (ret) {
> + dev_err(priv->dev, "error
Re: [GIT PULL] Documentation for 5.0
On Fri, Dec 28, 2018 at 8:06 AM Jonathan Corbet wrote: > > git://git.lwn.net/linux.git tags/docs-5.0 New signing key? And one that you forgot to push out to keyservers? Linus
Re: How to force RC to forward p2p TLPs
On 2018-12-28 7:29 p.m., Bjorn Helgaas wrote: >> We use the p2p DMA to transfer data between these two endpoint SOCs, >> and if the host server is not enable ACS in BIOS, the p2p works well, >> but when ACS is enabled in BIOS, the p2p is always failed. With the >> help of a protocol analyzer, we can see that the TLP is redirected to >> RC, and RC just discard it. >> >> I tried to find how to make RC forward redirected TLP to its original >> target, but nothing found, it seems this is highly related to the RC >> vendors. As far as I know, this is not possible with any RCs I've worked with. For our work, we ensure we disable ACS on paths between peers that want to communicate. This is why we introduced the 'disable_acs_redir' kernel parameter[1] to selectively disable ACS for bridges along P2P paths. >> So is there some spec or document to describe how to set the RC? Any >> suggestion is appreciated. If there is, it would be the documentation for your specific RC. Odds are, the RC simply does not support this. Logan [1] https://patchwork.kernel.org/patch/10549279/
KASAN: stack-out-of-bounds Write in ax25_getname
Hello, syzbot found the following crash on: HEAD commit:8fe28cb58bcb Linux 4.20 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=1604d02d40 kernel config: https://syzkaller.appspot.com/x/.config?x=7d581260bae0899a dashboard link: https://syzkaller.appspot.com/bug?extid=6a29097222b4d3b8617c compiler: gcc (GCC) 8.0.1 20180413 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=114a9ec340 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+6a29097222b4d3b86...@syzkaller.appspotmail.com IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready 8021q: adding VLAN 0 to HW filter on device team0 == BUG: KASAN: stack-out-of-bounds in memset include/linux/string.h:337 [inline] BUG: KASAN: stack-out-of-bounds in ax25_getname+0x58/0x790 net/ax25/af_ax25.c:1399 Write of size 72 at addr 8881d8547b80 by task syz-executor0/8181 CPU: 0 PID: 8181 Comm: syz-executor0 Not tainted 4.20.0 #166 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 check_memory_region_inline mm/kasan/kasan.c:260 [inline] check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267 memset+0x23/0x40 mm/kasan/kasan.c:285 memset include/linux/string.h:337 [inline] ax25_getname+0x58/0x790 net/ax25/af_ax25.c:1399 get_raw_socket drivers/vhost/net.c:1397 [inline] get_socket drivers/vhost/net.c:1453 [inline] vhost_net_set_backend drivers/vhost/net.c:1488 [inline] vhost_net_ioctl+0x139c/0x1bf0 drivers/vhost/net.c:1679 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457759 Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f25cd3fbc78 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 0003 RCX: 00457759 RDX: 20f1dff8 RSI: 4008af30 RDI: 0004 RBP: 0073bf00 R08: R09: R10: R11: 0246 R12: 7f25cd3fc6d4 R13: 004c1dd4 R14: 004d40e0 R15: The buggy address belongs to the page: page:ea00076151c0 count:0 mapcount:0 mapping: index:0x0 flags: 0x2fffc00() raw: 02fffc00 07610101 raw: page dumped because: kasan: bad access detected Memory state around the buggy address: 8881d8547a80: 00 f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f2 f2 f2 f2 8881d8547b00: 00 f2 f2 f2 f2 f2 f2 f2 04 f2 f2 f2 f2 f2 f2 f2 8881d8547b80: 00 00 00 00 00 00 04 f2 00 00 00 00 00 00 00 00 ^ 8881d8547c00: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 8881d8547c80: f2 f2 f2 f2 f2 f2 00 00 00 f2 f2 f2 f2 f2 00 00 == --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches
Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver
Hi! > >>With the "color" sysfs file it will make more sense to allow for user > >>defined color palettes. > >> > > > >I think defining these values in the device tree or acpi severely limits the > >devices > >capabilities. Especially in development phases. If the knobs were exposed > >then the user space > >can create new experiences. The color definition should be an absolute > >color defined in the dt and > >either the framework or user space needs to mix these appropriately. IMO > >user space should set the policy > >of the user experience and the dt/acpi needs to set the capabilities. > > > >I do like Pavels idea on defining the more standard binding pattern to > >"group" leds into a single group. > > > >Maybe the framework could take these groups and combine/group them into a > >single node with the groups colors. > > There is still HSV approach [0] in store. One problem with proposed > implementation is fixed algorithm of RGB <-> HSV color space conversion. > Maybe allowing for some board specific adjustments in DT would add > more flexibility. > > [0] https://lkml.org/lkml/2017/8/31/255 Yes we could do HSV. Problem is that that we do not really have RGB available. We do have integers for red, green and blue, but they do not correspond to RGB colorspace. Anyway, this should not be driver specific; all drivers should use one interface. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v8 01/25] scsi/atari_scsi: Don't select CONFIG_NVRAM
Christophe,
Am 30.12.2018 um 05:55 schrieb LEROY Christophe:
Michael Schmitz a écrit :
Hi Finn,
Am 29.12.2018 um 14:06 schrieb Finn Thain:
On Fri, 28 Dec 2018, LEROY Christophe wrote:
diff --git a/drivers/scsi/atari_scsi.c b/drivers/scsi/atari_scsi.c
index 89f5154c40b6..99e5729d910d 100644
--- a/drivers/scsi/atari_scsi.c
+++ b/drivers/scsi/atari_scsi.c
@@ -755,9 +755,10 @@ static int __init atari_scsi_probe(struct
platform_device *pdev)
if (ATARIHW_PRESENT(TT_SCSI) && setup_sg_tablesize >= 0)
atari_scsi_template.sg_tablesize = setup_sg_tablesize;
-if (setup_hostid >= 0) {
+if (setup_hostid >= 0)
atari_scsi_template.this_id = setup_hostid & 7;
-} else {
+#ifdef CONFIG_NVRAM
+else
Such ifdefs should be avoided in C files.
It would be better to use
} else if (IS_ENABLED(CONFIG_NVRAM)) {
I don't like #ifdefs either. However, as the maintainer of this file,
I am
okay with this one.
The old #ifdef CONFIG_NVRAM conditional compilation convention that gets
used here and under drivers/video/fbdev could probably be improved upon
but I consider this to be out-of-scope for this series, which is
complicated enough.
And as explained in the commit log, CONFIG_NVRAM=y and CONFIG_NVRAM=m
are
treaded differently by drivers. Therefore, IS_ENABLED would be
incorrect.
IS_BUILTIN(CONFIG_NVRAM) is probably what Christophe really meant to
suggest.
Doesn't #ifdef means either y or m ? So the same as IS_ENABLED() ?
#ifdef CONFIG_NVRAM is used if you want to match CONFIG_NVRAM=y. For
CONFIG_NVRAM=m, you'd use #ifdef CONFIG_NVRAM_MODULE.
Cheers,
Michael
Christophe
Or (really going out on a limb here):
IS_BUILTIN(CONFIG_NVRAM) ||
( IS_MODULE(CONFIG_ATARI_SCSI) && IS_ENABLED(CONFIG_NVRAM) )
Not that I'd advocate that, for this series.
Cheers,
Michael
KASAN: use-after-free Read in x25_device_event
Hello, syzbot found the following crash on: HEAD commit:38355a5f9a22 bnx2x: Fix NULL pointer dereference in bnx2x_.. git tree: net console output: https://syzkaller.appspot.com/x/log.txt?x=144e49ed40 kernel config: https://syzkaller.appspot.com/x/.config?x=7321a72d3309c029 dashboard link: https://syzkaller.appspot.com/bug?extid=04babcefcd396fabec37 compiler: gcc (GCC) 8.0.1 20180413 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+04babcefcd396fabe...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in x25_kill_by_device net/x25/af_x25.c:217 [inline] BUG: KASAN: use-after-free in x25_device_event+0x297/0x2b0 net/x25/af_x25.c:252 Read of size 8 at addr 8881b5c85ad0 by task syz-executor2/22350 CPU: 1 PID: 22350 Comm: syz-executor2 Not tainted 4.20.0-rc7+ #248 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433 x25_kill_by_device net/x25/af_x25.c:217 [inline] x25_device_event+0x297/0x2b0 net/x25/af_x25.c:252 notifier_call_chain+0x17e/0x380 kernel/notifier.c:93 __raw_notifier_call_chain kernel/notifier.c:394 [inline] raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401 call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1733 call_netdevice_notifiers net/core/dev.c:1751 [inline] __dev_notify_flags+0x29b/0x480 net/core/dev.c:7547 dev_change_flags+0xfd/0x150 net/core/dev.c:7581 dev_ifsioc+0x7d6/0xa80 net/core/dev_ioctl.c:237 dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:488 sock_do_ioctl+0x1f6/0x420 net/socket.c:973 sock_ioctl+0x313/0x690 net/socket.c:1074 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457759 Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f4c06d4fc78 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 0003 RCX: 00457759 RDX: 25c0 RSI: 8914 RDI: 0005 RBP: 0073bfa0 R08: R09: R10: R11: 0246 R12: 7f4c06d506d4 R13: 004c2c32 R14: 004d52f8 R15: Allocated by task 10606: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620 kmalloc include/linux/slab.h:546 [inline] x25_link_device_up+0xb2/0x500 net/x25/x25_link.c:249 x25_device_event+0x116/0x2b0 net/x25/af_x25.c:242 notifier_call_chain+0x17e/0x380 kernel/notifier.c:93 __raw_notifier_call_chain kernel/notifier.c:394 [inline] raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401 call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1733 call_netdevice_notifiers net/core/dev.c:1751 [inline] __dev_notify_flags+0x17a/0x480 net/core/dev.c:7545 dev_change_flags+0xfd/0x150 net/core/dev.c:7581 dev_ifsioc+0x7d6/0xa80 net/core/dev_ioctl.c:237 dev_ioctl+0x1b5/0xcc0 net/core/dev_ioctl.c:488 sock_do_ioctl+0x1f6/0x420 net/socket.c:973 sock_ioctl+0x313/0x690 net/socket.c:1074 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 22299: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 __cache_free mm/slab.c:3498 [inline] kfree+0xcf/0x230 mm/slab.c:3817 x25_neigh_put include/net/x25.h:253 [inline] __x25_remove_neigh+0x233/0x310 net/x25/x25_link.c:290 x25_link_device_down+0xc7/0x130 net/x25/x25_link.c:308 x25_device_event+0x262/0x2b0 net/x25/af_x25.c:254 notifier_call_chain+0x17e/0x380 kernel/notifier.c:93 __raw_notifier_call_chain kernel/notifier.c:394 [inline]
[PATCH] input_event: Provide override for sparc64
The usec part of the timeval is defined as
__kernel_suseconds_ttv_usec; /* microseconds */
Arnd noticed that sparc64 is the only architecture
that defines __kernel_suseconds_t as int rather than long.
This breaks the current y2038 fix for kernel as we only
access and define the timeval struct for non-kernel use cases.
But, this was hidden by an another typo in the use of __KERNEL__
qualifier.
Fix the typo, and provide an override for sparc64.
Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 on
32 bit systems")
Reported-by: Arnd Bergmann
Signed-off-by: Deepa Dinamani
---
include/uapi/linux/input.h | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index fb78f6f500f3..ffab958bc512 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -26,13 +26,17 @@
*/
struct input_event {
-#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) &&
!defined(__KERNEL)
+#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) &&
!defined(__KERNEL__)
struct timeval time;
#define input_event_sec time.tv_sec
#define input_event_usec time.tv_usec
#else
__kernel_ulong_t __sec;
+#ifdef CONFIG_SPARC64
+ unsigned int __usec;
+#else
__kernel_ulong_t __usec;
+#endif
#define input_event_sec __sec
#define input_event_usec __usec
#endif
--
2.17.1
Re: [GIT PULL] security: general updates for v4.21
On 12/28/2018 8:15 PM, Linus Torvalds wrote: > On Fri, Dec 28, 2018 at 8:09 PM James Morris wrote: >> Yep, I understand what you mean. I can't find the discussion from several >> years ago, but developers asked to be able to work with more current >> kernels, and I recall you saying that if you want to do this, merge to a >> specific -rc tag at least. > If people really want it, maybe the merge can state that explicit > thing, as it is I'm trying to push back on empty merges that don't > explain why they even exist. > > Linus The security tree tends to get changed from multiple directions, most of which aren't based out of the security sub-system. The mount rework from David is an excellent example. It gets hit just about any time there's a significant VFS or networking change. Keeping it current has helped find issues much earlier in the process.
Re: [PATCH] doc: net: fix bad references to network drivers
On Sat, Dec 29, 2018 at 07:05:18PM +0100, Otto Sabart wrote:
> Fix "reference to nonexisting document" warnings.
>
> Signed-off-by: Otto Sabart
Fixes: b255e500c8dc ("net: documentation: build a directory structure for
drivers")
Re: [PATCH 2/2] dt-bindings: edac: Aspeed AST2500
Hello Rob,
> From: Rob Herring
>
> On Sun, Dec 16, 2018 at 10:01:57PM -0800, Stefan Schaeckeler wrote:
> > From: Stefan M Schaeckeler
> >
> > Add support for the Aspeed AST2500 SoC EDAC driver.
> >
> > Signed-off-by: Stefan M Schaeckeler
> > ---
> > .../bindings/edac/aspeed-sdram-edac.txt | 34 +++
> > 1 file changed, 34 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> >
> > diff --git a/Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> > b/Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> > new file mode 100644
> > index ..57ba852883c7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/edac/aspeed-sdram-edac.txt
> > @@ -0,0 +1,34 @@
> > +Aspeed AST2500 SoC EDAC device driver
>
> Bindings are for h/w, not drivers
Changed "device driver" to "node".
I will also change the commit message to perhaps "Add support for EDAC on the
Aspeed AST2500 SoC."
> > +The Aspeed AST2500 SoC supports DDR3 and DDR4 memory with and without ECC
> > (error
> > +correction check).
> > +
> > +The memory controller supports SECDED (single bit error correction, double
> > bit
> > +error detection) and single bit error auto scrubbing by reserving 8 bits
> > for
> > +every 64 bit word (effectively reducing available memory to 8/9).
> > +
> > +First, ECC must be configured in u-boot. Then, this driver will expose
> > error
> > +counters via the edac kernel framework.
>
> Please reword this to not be u-boot or kernel specific.
The previous paragraph is now: Note, the bootloader must configure ECC mode in
the memory controller.
> Maybe this node is enabled in the bootloader or the OS can just read the
> registers to see if ECC is enabled. The latter is more future proof if
> you need to access the DDR ctrl registers for other reasons.
The driver's probe function has a sanity check. It consults the memory
controller for enabled ECC mode:
/* bail out if ECC mode is not configured */
regmap_read(aspeed_edac_regmap, ASPEED_MCR_CONF, );
if (!(reg04 & ASPEED_MCR_CONF_ECC)) {
dev_err(>dev, "ECC mode is not configured in u-boot\n");
return -EPERM;
}
> > +A note on memory organization in ECC mode: every 512 bytes are followed by
> > 64
> > +bytes of ECC codes.
>
> That sounds strange. Normally, the memory would be 72-bits wide to hold
> the ECC byte for each 64-bit chunk. It would be inefficient to access
> the ECC byte in a discontiguous location.
When a word is loaded from memory, the corresponding ECC word needs to be
loaded as well (both words can and will be cached). Performance relies on
temporal and spatial locality. That can fire back, of course.
> In any case, none of this is really important for the binding.
I will move above and below paragraph into Kconfig.
> > The address remapping is done in hardware and is fully
> > +transparent to firmware and software. Because of this, ECC mode must be
> > +configured in u-boot as part of the memory initialization as one can not
> > switch
> > +from one mode to another when executing in memory.
> > +
> > +
> > +
> > +Required properties:
> > +- compatible: should be "aspeed,ast2500-sdram-edac"
> > +- reg:sdram controller register set should be <0x1e6e 0x174>
> > +- interrupts: should be AVIC interrupt #0
> > +
> > +
> > +Example:
> > +
> > + edac: sdram@1e6e {
> > + compatible = "aspeed,ast2500-sdram-edac";
> > + reg = <0x1e6e 0x174>;
> > + interrupts = <0>;
> > + status = "okay";
>
> Don't show status in examples.
Removed.
>
> > + };
> > --
> > 2.19.1
To wrap it up, for the next patchset, I will generate a diff for below text
- - - snip - - -
Aspeed AST2500 SoC EDAC node
The Aspeed AST2500 SoC supports DDR3 and DDR4 memory with and without ECC (error
correction check).
The memory controller supports SECDED (single bit error correction, double bit
error detection) and single bit error auto scrubbing by reserving 8 bits for
every 64 bit word (effectively reducing available memory to 8/9).
Note, the bootloader must configure ECC mode in the memory controller.
Required properties:
- compatible: should be "aspeed,ast2500-sdram-edac"
- reg:sdram controller register set should be <0x1e6e 0x174>
- interrupts: should be AVIC interrupt #0
Example:
edac: sdram@1e6e {
compatible = "aspeed,ast2500-sdram-edac";
reg = <0x1e6e 0x174>;
interrupts = <0>;
};
- - - snip - - -
Stefan
Re: [PATCH] tools headers: move the nolibc header from rcutorture to tools/include/nolibc/
On Sat, Dec 29, 2018 at 10:25:08AM -0800, Paul E. McKenney wrote: > On Sat, Dec 29, 2018 at 07:04:53PM +0100, Willy Tarreau wrote: > > As suggested by Ingo, this header file might benefit other tools than > > just rcutorture. For now it's quite limited, but is easy to extend, so > > exposing it into tools/include/nolibc/ will make it much easier to > > adopt by other tools. > > > > The mkinitrd.sh script in rcutorture was updated to use this new location. > > > > Cc: Ingo Molnar > > Cc: Arnaldo Carvalho de Melo > > Cc: Paul E. McKenney > > Signed-off-by: Willy Tarreau > > Thank you, Willy! You're welcome! > I have queued all four of these. Thanks! > Should there > be a MAINTAINERS file entry for the new include/nolibc home for this > library code? Good idea, I didn't think about it. Yes, I can add one and will send another patch. In case that helps I've created a repo at /pub/scm/linux/kernel/git/wtarreau/nolibc.git Cheers, Willy
Re: [PATCH 2/2] leds: lp5024: Add the LP5024/18 RGB LED driver
On 12/21/18 2:05 PM, Dan Murphy wrote:
On 12/21/2018 01:32 AM, Jacek Anaszewski wrote:
On 12/20/18 9:31 PM, Jacek Anaszewski wrote:
On 12/19/18 10:50 PM, Dan Murphy wrote:
On 12/19/2018 03:36 PM, Jacek Anaszewski wrote:
Hi Dan and Pavel,
On 12/19/18 9:41 PM, Dan Murphy wrote:
Pavel
On 12/19/2018 02:10 PM, Pavel Machek wrote:
On Wed 2018-12-19 13:41:18, Dan Murphy wrote:
Pavel
Thanks for the review.
On 12/19/2018 01:34 PM, Pavel Machek wrote:
Hi!
+static DEVICE_ATTR_WO(ctrl_bank_a_mix);
+static DEVICE_ATTR_WO(ctrl_bank_b_mix);
+static DEVICE_ATTR_WO(ctrl_bank_c_mix);
+
+static struct attribute *lp5024_ctrl_bank_attrs[] = {
+ _attr_ctrl_bank_a_mix.attr,
+ _attr_ctrl_bank_b_mix.attr,
+ _attr_ctrl_bank_c_mix.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lp5024_ctrl_bank);
...
+
+static DEVICE_ATTR_WO(led1_mix);
+static DEVICE_ATTR_WO(led2_mix);
+static DEVICE_ATTR_WO(led3_mix);
+
+static struct attribute *lp5024_led_independent_attrs[] = {
+ _attr_led1_mix.attr,
+ _attr_led2_mix.attr,
+ _attr_led3_mix.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(lp5024_led_independent);
Ok, so you are adding new sysfs files. Are they _really_ neccessary?
We'd like to have uniform interfaces for the leds.
Yes I am adding these for this driver.
They adjust the individual brightness of each LED connected (what the HW guys
called mixing).
The standard brightness sysfs adjusts all 3 LEDs simultaneously so that all 3
LEDs brightness are
uniform.
1 LED, 1 brightness file... that's what we do. Why should this one be different?
Yes I understand this and 1 DT child node per LED out but...
This device has a single register to control 3 LEDs brightness as a group and
individual brightness
registers to control the LEDs independently to mix the LEDs to a specific color.
There needs to be a way to control both so that developers can mix and adjust
the individual RGB and
then control the brightness of the group during run time without touching the
"mixing" value.
I don't think a user needs nor would want to have 24 different LED nodes with
24 different brightness files.
Or with the LP5036 that would have 36 LED nodes.
Table 1 in the data sheet shows how the outputs map to the control banks to the
LED registers.
Some time ago we had discussion with Vesa Jääskeläinen about possible
approaches to RGB LEDs [0]. What seemed to be the most suitable
variation of the discussed out-of-tree approach was the "color" property
and array of color triplets defined in Device Tree per each color.
Why does Device tree define the color?
Rob indicated that Device tree is supposed to define the hardware.
This thread seems to be defining the operation.
Perceived colors produced by LEDs from different manufacturers may
differ and this alone should be deemed a sufficient argument for having
board specific color definitions.
Shouldn't the color be done via user space and not dt?
I think that we should keep the userspace interface as simple
as possible and backwards compatible with monochrome LEDs.
I also propose to avoid the introduction of a color sysfs
property in favor of creating separate LED class devices
for different "color ranges". The devices would drive the same
LED but using different preset color levels.
On the other hand, scattering the control over the hardware
among multiple LED class devices would complicate extension
of pattern trigger with the support for RGB LEDs.
I looks like we will need the "color" sysfs file anyway.
We don't have to expose all device knobs to the userspace,
but instead provide some predefined configurations. It would
improve user experience by keeping LED class devices simple
in use. It would be Device Tree designer's responsibility to
provide color definitions that make sense for given RGB LED
controller and RGB LED element configuration.
Registering color palette with devm_rgb_register() you proposed
is also an option, but with one LED class device per color palette
it would mean allowing for creation/destruction of LED class
devices by any user having access to given LED's sysfs interface,
which is really bad solution.
With the "color" sysfs file it will make more sense to allow for user
defined color palettes.
I think defining these values in the device tree or acpi severely limits the
devices
capabilities. Especially in development phases. If the knobs were exposed
then the user space
can create new experiences. The color definition should be an absolute color
defined in the dt and
either the framework or user space needs to mix these appropriately. IMO user
space should set the policy
of the user experience and the dt/acpi needs to set the capabilities.
I do like Pavels idea on defining the more standard binding pattern to "group"
leds into a single group.
Maybe the framework could take these groups and combine/group them into a
single node with the groups colors.
There is still HSV approach [0] in store. One problem with proposed
Re: [PATCH] tools headers: move the nolibc header from rcutorture to tools/include/nolibc/
On Sat, Dec 29, 2018 at 07:04:53PM +0100, Willy Tarreau wrote:
> As suggested by Ingo, this header file might benefit other tools than
> just rcutorture. For now it's quite limited, but is easy to extend, so
> exposing it into tools/include/nolibc/ will make it much easier to
> adopt by other tools.
>
> The mkinitrd.sh script in rcutorture was updated to use this new location.
>
> Cc: Ingo Molnar
> Cc: Arnaldo Carvalho de Melo
> Cc: Paul E. McKenney
> Signed-off-by: Willy Tarreau
Thank you, Willy! I have queued all four of these. Should there
be a MAINTAINERS file entry for the new include/nolibc home for this
library code?
Thanx, Paul
> ---
> tools/{testing/selftests/rcutorture/bin => include/nolibc}/nolibc.h | 0
> tools/testing/selftests/rcutorture/bin/mkinitrd.sh | 4 ++--
> 2 files changed, 2 insertions(+), 2 deletions(-)
> rename tools/{testing/selftests/rcutorture/bin => include/nolibc}/nolibc.h
> (100%)
>
> diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
> b/tools/include/nolibc/nolibc.h
> similarity index 100%
> rename from tools/testing/selftests/rcutorture/bin/nolibc.h
> rename to tools/include/nolibc/nolibc.h
> diff --git a/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
> b/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
> index da29839..d93bca1 100755
> --- a/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
> +++ b/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
> @@ -124,8 +124,8 @@ if echo -e "#if
> __x86_64__||__i386__||__i486__||__i586__||__i686__" \
> | grep -q '^yes'; then
> # architecture supported by nolibc
> ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
> - -nostdlib -include ../bin/nolibc.h -lgcc -s -static -Os \
> - -o init init.c
> + -nostdlib -include ../../../../include/nolibc/nolibc.h \
> + -lgcc -s -static -Os -o init init.c
> else
> ${CROSS_COMPILE}gcc -s -static -Os -o init init.c
> fi
> --
> 2.9.0
>
Re: [PATCH v2] media: venus: add debugfs support
Hi Malathi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20181224]
[cannot apply to v4.20]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Malathi-Gottam/media-venus-add-debugfs-support/20181228-172634
base: git://linuxtv.org/media_tree.git master
config: microblaze-allyesconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=microblaze
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:14,
from include/linux/clk.h:16,
from drivers/media/platform/qcom/venus/helpers.c:15:
drivers/media/platform/qcom/venus/helpers.c: In function
'venus_helper_check_codec':
>> drivers/media/platform/qcom/venus/helpers.c:78:40: error: 'pixfmt'
>> undeclared (first use in this function); did you mean 'pr_fmt'?
dprintk(WARN, "Unknown format:%x\n", pixfmt);
^~
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/media/platform/qcom/venus/helpers.c:78:3: note: in expansion of
macro 'dprintk'
dprintk(WARN, "Unknown format:%x\n", pixfmt);
^~~
drivers/media/platform/qcom/venus/helpers.c:78:40: note: each undeclared
identifier is reported only once for each function it appears in
dprintk(WARN, "Unknown format:%x\n", pixfmt);
^~
include/linux/printk.h:315:34: note: in definition of macro 'pr_info'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^~~
drivers/media/platform/qcom/venus/helpers.c:78:3: note: in expansion of
macro 'dprintk'
dprintk(WARN, "Unknown format:%x\n", pixfmt);
^~~
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:14,
from include/linux/clk.h:16,
from drivers/media/platform/qcom/venus/helpers.c:15:
drivers/media/platform/qcom/venus/helpers.c: In function
'venus_helper_queue_dpb_bufs':
include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of
type 'int', but argument 3 has type 'const char *' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~
include/linux/kern_levels.h:14:19: note: in expansion of macro 'KERN_SOH'
#define KERN_INFO KERN_SOH "6" /* informational */
^~~~
include/linux/printk.h:315:9: note: in expansion of macro 'KERN_INFO'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^
drivers/media/platform/qcom/venus/core.h:55:4: note: in expansion of macro
'pr_info'
pr_info("venus:" fmt, \
^~~
drivers/media/platform/qcom/venus/helpers.c:107:4: note: in expansion of
macro 'dprintk'
dprintk(ERR, "%s: Failed to queue dpb buf to hfi: %d\n",
^~~
drivers/media/platform/qcom/venus/helpers.c:107:55: note: format string is
defined here
dprintk(ERR, "%s: Failed to queue dpb buf to hfi: %d\n",
~^
%s
In file included from include/linux/printk.h:7,
from include/linux/kernel.h:14,
from include/linux/clk.h:16,
from drivers/media/platform/qcom/venus/helpers.c:15:
include/linux/kern_levels.h:5:18: warning: too many arguments for format
[-Wformat-extra-args]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~
include/linux/kern_levels.h:14:19: note: in expansion of macro 'KERN_SOH'
#define KERN_INFO KERN_SOH "6" /* informational */
^~~~
include/linux/printk.h:315:9: note: in expansion of macro 'KERN_INFO'
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^
drivers/media/platform/qcom/venus/core.h:55:4: note: in expansion of macro
'pr_info'
pr_info("venus:" fmt, \
^~~
drivers/media/platform/qcom/venus/helpers.c:107:4: note: in expansion of
macro 'dprintk'
dprintk(ERR, "%s: Failed to queue dpb buf to hfi: %d\n",
^~~
drivers/media/platform/qcom/venus/helpers.c: In function
'venus_helper_alloc_dpb_bufs':
include/linux/kern_levels.h:5:18: warning: format '%d' expects argument of
type 'int', but argument 2 has type 'char *' [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
Re: [PULL 00/25] Xtensa updates for v4.21
The pull request you sent on Fri, 28 Dec 2018 10:31:20 -0800: > git://github.com/jcmvbkbc/linux-xtensa.git tags/xtensa-20181228 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/9ef10340749e1da0c7fde609cedd5360f8484a0b Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [GIT PULL] nds32 new features and bug fix for 4.21
The pull request you sent on Fri, 28 Dec 2018 16:24:01 +0800: > ssh://g...@gitolite.kernel.org/pub/scm/linux/kernel/git/greentime/linux.git > tags/nds32-for-linus-4.21 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/889bb74302e5aba85d987b4093344150984d7cda Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
[PATCH] doc: net: fix bad references to network drivers
Fix "reference to nonexisting document" warnings. Signed-off-by: Otto Sabart --- Documentation/networking/index.rst | 26 +- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst index 6a47629ef8ed..59e86de662cd 100644 --- a/Documentation/networking/index.rst +++ b/Documentation/networking/index.rst @@ -11,19 +11,19 @@ Contents: batman-adv can can_ucan_protocol - dpaa2/index - e100 - e1000 - e1000e - fm10k - igb - igbvf - ixgb - ixgbe - ixgbevf - i40e - iavf - ice + device_drivers/freescale/dpaa2/index + device_drivers/intel/e100 + device_drivers/intel/e1000 + device_drivers/intel/e1000e + device_drivers/intel/fm10k + device_drivers/intel/igb + device_drivers/intel/igbvf + device_drivers/intel/ixgb + device_drivers/intel/ixgbe + device_drivers/intel/ixgbevf + device_drivers/intel/i40e + device_drivers/intel/iavf + device_drivers/intel/ice kapi z8530book msg_zerocopy -- 2.17.2 signature.asc Description: PGP signature
Re: [GIT PULL] libnvdimm for 4.21
On Sat, Dec 29, 2018 at 7:36 AM Konstantin Ryabitsev wrote: > > On Fri, Dec 28, 2018 at 09:57:28PM -0800, Linus Torvalds wrote: > > Hmm. > > > > This pull request doesn't seem to have gotten an automatic pr-tracker > > reply, even though I pulled it, and even though it was cc'd to lkml. > > > > Konstantin, any idea why the automation didn't trigger? I'm not seeing > > anything all that odd about it. > > As you've noticed, it's not on lore, and it's also not on any other LKML > archive out there (I've checked lkml.org, lkml.iu.edu, marc.info). So, > I'm not sure precisely what happened, but it would appear that despite > the CC in the headers, the mail never actually passed vger. > > > Does lkml hate Dan? I suspect this is the case, or more specifically this is likely one of those times where vger needed to put the Intel mail servers in a time-out for bouncing too many mails.
[PATCH] tools headers: move the nolibc header from rcutorture to tools/include/nolibc/
As suggested by Ingo, this header file might benefit other tools than
just rcutorture. For now it's quite limited, but is easy to extend, so
exposing it into tools/include/nolibc/ will make it much easier to
adopt by other tools.
The mkinitrd.sh script in rcutorture was updated to use this new location.
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Paul E. McKenney
Signed-off-by: Willy Tarreau
---
tools/{testing/selftests/rcutorture/bin => include/nolibc}/nolibc.h | 0
tools/testing/selftests/rcutorture/bin/mkinitrd.sh | 4 ++--
2 files changed, 2 insertions(+), 2 deletions(-)
rename tools/{testing/selftests/rcutorture/bin => include/nolibc}/nolibc.h
(100%)
diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
b/tools/include/nolibc/nolibc.h
similarity index 100%
rename from tools/testing/selftests/rcutorture/bin/nolibc.h
rename to tools/include/nolibc/nolibc.h
diff --git a/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
b/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
index da29839..d93bca1 100755
--- a/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
+++ b/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
@@ -124,8 +124,8 @@ if echo -e "#if
__x86_64__||__i386__||__i486__||__i586__||__i686__" \
| grep -q '^yes'; then
# architecture supported by nolibc
${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
- -nostdlib -include ../bin/nolibc.h -lgcc -s -static -Os \
- -o init init.c
+ -nostdlib -include ../../../../include/nolibc/nolibc.h \
+ -lgcc -s -static -Os -o init init.c
else
${CROSS_COMPILE}gcc -s -static -Os -o init init.c
fi
--
2.9.0
[PATCH 3/4] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
Ingo rightfully asked for a bit more documentation in the nolibc header, so this patch adds some explanation about its purpose, how it's made, and how to use it. Cc: Ingo Molnar Cc: Paul E. McKenney Cc: Randy Dunlap Signed-off-by: Willy Tarreau --- tools/testing/selftests/rcutorture/bin/nolibc.h | 90 + 1 file changed, 78 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h b/tools/testing/selftests/rcutorture/bin/nolibc.h index 985364c..6643ba9 100644 --- a/tools/testing/selftests/rcutorture/bin/nolibc.h +++ b/tools/testing/selftests/rcutorture/bin/nolibc.h @@ -3,6 +3,84 @@ * Copyright (C) 2017-2018 Willy Tarreau */ +/* + * This file is designed to be used as a libc alternative for minimal programs + * with very limited requirements. It consists of a small number of syscall and + * type definitions, and the minimal startup code needed to call main(). + * All syscalls are declared as static functions so that they can be optimized + * away by the compiler when not used. + * + * Syscalls are split into 3 levels: + * - the lower level is the arch-specific syscall() definition, consisting in + * assembly code in compound expressions. These are called my_syscall0() to + * my_syscall6() depending on the number of arguments. The MIPS + * implementation is limited to 5 arguments. All input arguments are cast + * to a long stored in a register. These expressions always return the + * syscall's return value as a signed long value which is often either a + * pointer or the negated errno value. + * + * - the second level is mostly architecture-independent. It is made of + * static functions called sys_() which rely on my_syscallN() + * depending on the syscall definition. These functions are responsible + * for exposing the appropriate types for the syscall arguments (int, + * pointers, etc) and for setting the appropriate return type (often int). + * A few of them are architecture-specific because the syscalls are not all + * mapped exactly the same among architectures. For example, some archs do + * not implement select() and need pselect6() instead, so the sys_select() + * function will have to abstract this. + * + * - the third level is the libc call definition. It exposes the lower raw + * sys_() calls in a way that looks like what a libc usually does, + * takes care of specific input values, and of setting errno upon error. + * There can be minor variations compared to standard libc calls. For + * example the open() call always takes 3 args here. + * + * The errno variable is declared static and unused. This way it can be + * optimized away if not used. However this means that a program made of + * multiple C files may observe different errno values (one per C file). For + * the type of programs this project targets it usually is not a problem. The + * resulting program may even be reduced by defining the NOLIBC_IGNORE_ERRNO + * macro, in which case the errno value will never be assigned. + * + * Some stdint-like integer types are defined. These are valid on all currently + * supported architectures, because signs are enforced, ints are assumed to be + * 32 bits, longs the size of a pointer and long long 64 bits. If more + * architectures have to be supported, this may need to be adapted. + * + * Some macro definitions like the O_* values passed to open(), and some + * structures like the sys_stat struct depend on the architecture. + * + * The definitions start with the architecture-specific parts, which are picked + * based on what the compiler knows about the target architecture, and are + * completed with the generic code. Since it is the compiler which sets the + * target architecture, cross-compiling normally works out of the box without + * having to specify anything. + * + * Finally some very common libc-level functions are provided. It is the case + * for a few functions usually found in string.h, ctype.h, or stdlib.h. Nothing + * is currently provided regarding stdio emulation. + * + * The macro NOLIBC is always defined, so that it is possible for a program to + * check this macro to know if it is being built against and decide to disable + * some features or simply not to include some standard libc files. + * + * Ideally this file should be split in multiple files for easier long term + * maintenance, but provided as a single file as it is now, it's quite + * convenient to use. Maybe some variations involving a set of includes at the + * top could work. + * + * A simple static executable may be built this way : + * $ gcc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib \ + *-static -include nolibc.h -lgcc -o hello hello.c + * + * A very useful calling convention table may be found here : + * http://man7.org/linux/man-pages/man2/syscall.2.html + * + * This doc is quite convenient though not necessarily up to date : +
[PATCH 2/4] rcutorture/nolibc: fix some poor indentation and alignment
A few macros had their rightmost backslash misaligned, and the pollfd
struct definition resisted the previous code reindent. Nothing else
changed.
Cc: Paul E. McKenney
Signed-off-by: Willy Tarreau
---
tools/testing/selftests/rcutorture/bin/nolibc.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
b/tools/testing/selftests/rcutorture/bin/nolibc.h
index 30bd27b..985364c 100644
--- a/tools/testing/selftests/rcutorture/bin/nolibc.h
+++ b/tools/testing/selftests/rcutorture/bin/nolibc.h
@@ -81,9 +81,9 @@ typedef signed longtime_t;
/* for poll() */
struct pollfd {
-int fd;
-short int events;
-short int revents;
+ int fd;
+ short int events;
+ short int revents;
};
/* for select() */
@@ -239,7 +239,7 @@ struct stat {
"syscall\n" \
: "=a" (_ret) \
: "0"(_num) \
- : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
\
+ : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
);\
_ret; \
})
@@ -255,7 +255,7 @@ struct stat {
: "=a" (_ret) \
: "r"(_arg1), \
"0"(_num) \
- : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
\
+ : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
);\
_ret; \
})
@@ -272,7 +272,7 @@ struct stat {
: "=a" (_ret) \
: "r"(_arg1), "r"(_arg2), \
"0"(_num) \
- : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
\
+ : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
);\
_ret; \
})
@@ -290,7 +290,7 @@ struct stat {
: "=a" (_ret) \
: "r"(_arg1), "r"(_arg2), "r"(_arg3), \
"0"(_num) \
- : "rcx", "r8", "r9", "r10", "r11", "memory", "cc"
\
+ : "rcx", "r8", "r9", "r10", "r11", "memory", "cc" \
);\
_ret; \
})
--
2.9.0
[PATCH 1/4] rcutorture/nolibc: fix the clobbered registers in the MIPS syscall definition
A last-minute checkpatch cleanup caused most of list of clobbered
registers to be lost in the MIPS syscall definition. As it is right
now the code is not used on MIPS, but it's better to fix it before
it gets used.
Cc: Paul E. McKenney
Signed-off-by: Willy Tarreau
---
tools/testing/selftests/rcutorture/bin/nolibc.h | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/rcutorture/bin/nolibc.h
b/tools/testing/selftests/rcutorture/bin/nolibc.h
index f98f5b9..30bd27b 100644
--- a/tools/testing/selftests/rcutorture/bin/nolibc.h
+++ b/tools/testing/selftests/rcutorture/bin/nolibc.h
@@ -1006,7 +1006,7 @@ struct sys_stat_struct {
: "=r"(_num), "=r"(_arg4) \
: "r"(_num) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
@@ -1025,7 +1025,7 @@ struct sys_stat_struct {
: "0"(_num), \
"r"(_arg1) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
@@ -1045,7 +1045,7 @@ struct sys_stat_struct {
: "0"(_num), \
"r"(_arg1), "r"(_arg2) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
@@ -1066,7 +1066,7 @@ struct sys_stat_struct {
: "0"(_num), \
"r"(_arg1), "r"(_arg2), "r"(_arg3) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
@@ -1087,7 +1087,7 @@ struct sys_stat_struct {
: "0"(_num), \
"r"(_arg1), "r"(_arg2), "r"(_arg3), "r"(_arg4) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
@@ -1110,7 +1110,7 @@ struct sys_stat_struct {
: "0"(_num), \
"r"(_arg1), "r"(_arg2), "r"(_arg3), "r"(_arg4), "r"(_arg5) \
: "memory", "cc", "at", "v1", "hi", "lo", \
- \
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "t7", "t8", "t9" \
);\
_arg4 ? -_num : _num; \
})
--
2.9.0
[PATCH-v2 0/3] rcutorture: minor nolibc fixes
While working on adding some documentation to the nolibc header provided with rcutorture, I noticed a few accidently deleted lines losing clobbered registers and some leftover spaces that I fixed. In addition, I finally added some documentation to the file, as requested by Ingo. v2: fixed some spelling mistakes after Randy's review Willy Tarreau (3): rcutorture/nolibc: fix the clobbered registers in the MIPS syscall definition rcutorture/nolibc: fix some poor indentation and alignment rcutorture/nolibc: add a bit of documentation to explain how to use nolibc tools/testing/selftests/rcutorture/bin/nolibc.h | 116 +++- 1 file changed, 91 insertions(+), 25 deletions(-) -- 2.9.0
Re: [PATCH 3/3] rcutorture/nolibc: add a bit of documentation to explain how to use nolibc
Hi Randy, On Sat, Dec 29, 2018 at 08:50:09AM -0800, Randy Dunlap wrote: > This is a good summary IMO. Thanks. > And it's in good shape -- doesn't *require* any fixes. > But if you do make any changes to it, here are a few suggestions. :) Thanks very much. > > + * This file is designed to be used as a libc alternative for minimal > > programs > > + * with very limited requirements. It consists of a small number of > > syscall and > > + * types definitions, and the minimal startup code needed to call main(). > > type Funny, I hesitated on this one and "fixed" it :-) > > + * All syscalls are declared as static functions so that they can be > > optimized > > + * away by the compiler when not used. > > + * > > + * Syscalls are split between 3 levels : > > Instead of "between", use either "among" or "into". and then "levels:". Will do, thanks. > > + * - the lower level is the arch-specific syscall() definition, > > consisting in > > + * assembly code in compound expressions. These ones are called > > Apparently "these ones" is acceptable in UK English, not so in US English. Oh I didn't know, I've used it quite a bit in the last decades, thinking it was a valid plural for "this one". It seems like I should use "These" instead, feel free to suggest otherwise. > I don't like it, but we do accept UK English here. :) I prefer to be corrected and to avoid using bad English, whether it's US or UK, as much as I hate to make mistakes in French. > > + * Some stdint-like integer types are defined. These ones are valid on all > > These are valid on all OK, makes sense according to the point above. > ciao. thanks. I'm applying the changes right now to my local tree and will respin a version. Thank you! Willy
KASAN: use-after-free Read in delete_and_unsubscribe_port
Hello, syzbot found the following crash on: HEAD commit:8fe28cb58bcb Linux 4.20 git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=166859b340 kernel config: https://syzkaller.appspot.com/x/.config?x=7d581260bae0899a dashboard link: https://syzkaller.appspot.com/bug?extid=9917f000776fb73fa40f compiler: gcc (GCC) 8.0.1 20180413 (experimental) Unfortunately, I don't have any reproducer for this crash yet. IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+9917f000776fb73fa...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in __read_once_size include/linux/compiler.h:191 [inline] BUG: KASAN: use-after-free in list_empty include/linux/list.h:226 [inline] BUG: KASAN: use-after-free in delete_and_unsubscribe_port+0x5d7/0x6d0 sound/core/seq/seq_ports.c:548 Read of size 8 at addr 8881d1c05590 by task syz-executor1/6930 CPU: 0 PID: 6930 Comm: syz-executor1 Not tainted 4.20.0 #167 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1d3/0x2c6 lib/dump_stack.c:113 print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256 kasan_report_error mm/kasan/report.c:354 [inline] kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433 __read_once_size include/linux/compiler.h:191 [inline] list_empty include/linux/list.h:226 [inline] delete_and_unsubscribe_port+0x5d7/0x6d0 sound/core/seq/seq_ports.c:548 snd_seq_port_disconnect+0x17b/0x280 sound/core/seq/seq_ports.c:628 snd_seq_ioctl_unsubscribe_port+0x1d8/0x310 sound/core/seq/seq_clientmgr.c:1499 snd_seq_ioctl+0x253/0x440 sound/core/seq/seq_clientmgr.c:2138 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457759 Code: fd b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 cb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f36626cac78 EFLAGS: 0246 ORIG_RAX: 0010 RAX: ffda RBX: 0003 RCX: 00457759 RDX: 21c0 RSI: 40505331 RDI: 0003 RBP: 0073bf00 R08: R09: R10: R11: 0246 R12: 7f36626cb6d4 R13: 004ca358 R14: 004d35d0 R15: Allocated by task 6924: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620 kmalloc include/linux/slab.h:546 [inline] kzalloc include/linux/slab.h:741 [inline] snd_seq_port_connect+0xe0/0x760 sound/core/seq/seq_ports.c:571 snd_seq_ioctl_subscribe_port+0x1d8/0x310 sound/core/seq/seq_clientmgr.c:1458 snd_seq_ioctl+0x253/0x440 sound/core/seq/seq_clientmgr.c:2138 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 6931: save_stack+0x43/0xd0 mm/kasan/kasan.c:448 set_track mm/kasan/kasan.c:460 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 __cache_free mm/slab.c:3498 [inline] kfree+0xcf/0x230 mm/slab.c:3817 snd_seq_port_disconnect+0x205/0x280 sound/core/seq/seq_ports.c:632 snd_seq_ioctl_unsubscribe_port+0x1d8/0x310 sound/core/seq/seq_clientmgr.c:1499 snd_seq_ioctl+0x253/0x440 sound/core/seq/seq_clientmgr.c:2138 vfs_ioctl fs/ioctl.c:46 [inline] file_ioctl fs/ioctl.c:509 [inline] do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 __do_sys_ioctl fs/ioctl.c:720 [inline] __se_sys_ioctl fs/ioctl.c:718 [inline] __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe The buggy address belongs to the object at 8881d1c05540 which belongs to the cache kmalloc-128 of size 128 The buggy address is located 80 bytes inside of 128-byte region [8881d1c05540, 8881d1c055c0) The buggy address belongs to the page: page:ea0007470140 count:1 mapcount:0 mapping:8881da800640 index:0x8881d1c05b40 flags: 0x2fffc000200(slab) raw: 02fffc000200
Re: KASAN: use-after-free Read in ax25_fillin_cb
On Fri, Dec 28, 2018 at 02:51:04PM -0800, syzbot wrote:
> BUG: KASAN: use-after-free in ax25_fillin_cb_from_dev net/ax25/af_ax25.c:450
> [inline]
> BUG: KASAN: use-after-free in ax25_fillin_cb+0x6d5/0x810
> net/ax25/af_ax25.c:477
> Read of size 4 at addr 8881ccecc438 by task syz-executor5/11370
Not good... Why do things like this always pop up when I'm traveling?
> ax25_fillin_cb_from_dev net/ax25/af_ax25.c:450 [inline]
> ax25_fillin_cb+0x6d5/0x810 net/ax25/af_ax25.c:477
> ax25_setsockopt+0x92a/0xa20 net/ax25/af_ax25.c:663
That's here:
// ...
case SO_BINDTODEVICE:
// ...
dev = dev_get_by_name(_net, devname);
if (!dev) {
res = -ENODEV;
break;
}
ax25->ax25_dev = ax25_dev_ax25dev(dev);
ax25_fillin_cb(ax25, ax25->ax25_dev);
dev_put(dev);
break;
Thus, dev is not NULL, and neither is dev->ax25_ptr.
> Freed by task 11339:
> [..]
> ax25_dev_device_down+0x164/0x2f0 net/ax25/ax25_dev.c:129
> ax25_device_event+0x1f6/0x2e0 net/ax25/af_ax25.c:131
> notifier_call_chain+0x17e/0x380 kernel/notifier.c:93
ax25_dev_device_down() has this:
if ((s = ax25_dev_list) == ax25_dev) {
ax25_dev_list = s->next;
spin_unlock_bh(_dev_lock);
dev_put(dev);
kfree(ax25_dev);
return;
}
while (s != NULL && s->next != NULL) {
if (s->next == ax25_dev) {
s->next = ax25_dev->next;
spin_unlock_bh(_dev_lock);
dev_put(dev);
kfree(ax25_dev); // < here
return;
}
s = s->next;
}
spin_unlock_bh(_dev_lock);
dev->ax25_ptr = NULL;
I hope I didn't write this... *shudders*
Anyway, we are indeed missing to set dev->ax25_ptr to NULL if we're at
the head or somewhere on the ax25_dev_list, probably because it will be
cleaned up on removal of dev anyway. Unfortunately, in the meantime
either someone could call setsockopt() on an existing socket, or the
setsockopt() call got interrupted. From my POV the SO_BINDTODEVICE needs
to get protected by this:
spin_lock_bh(_dev_lock);
ax25->ax25_dev = ax25_dev_ax25dev(dev);
if (ax25->ax25_dev != NULL) {
ax25_fillin_cb(ax25, ax25->ax25_dev);
dev_put(dev);
}
spin_unlock_bh(_dev_lock);
and ax25_dev_device_down() needs to be rewritten and ensure
that dev->ax25_ptr gets set to NULL after each kfree(ax25_dev)
Unfortunately, I'm on a low bandwidth connection right now. I'd be
grateful if someone could create a patch. This is likely not a high
impact issue (unpriviliged users can't set up or tear down interfaces),
still it may cause hard to find memory corruptions.
- Joerg (yes, I'm still alive)
--
Joerg Reuterhttp://yaina.de/jreuter
And I make my way to where the warm scent of soil fills the evening air.
Everything is waiting quietly out there (Anne Clark)
[PATCH] nvme: pci: Use the same attributes when freeing host_mem_desc_bufs.
When using HMB the PCIe host driver allocates host_mem_desc_bufs using
dma_alloc_attrs() but frees them using dma_free_coherent(). Use the
correct dma_free_attrs() function to free the buffers.
Found out while doing some code inspection to figure out broken
NVMe support in linux-next-20181224. Without this change I'm getting
a kernel crash due to an invalid VM area being freed.
Signed-off-by: Liviu Dudau
---
drivers/nvme/host/pci.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c33bb201b8846..0f45868e8af98 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1748,8 +1748,9 @@ static void nvme_free_host_mem(struct nvme_dev *dev)
struct nvme_host_mem_buf_desc *desc = >host_mem_descs[i];
size_t size = le32_to_cpu(desc->size) * dev->ctrl.page_size;
- dma_free_coherent(dev->dev, size, dev->host_mem_desc_bufs[i],
- le64_to_cpu(desc->addr));
+ dma_free_attrs(dev->dev, size, dev->host_mem_desc_bufs[i],
+ le64_to_cpu(desc->addr),
+ DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_NO_WARN);
}
kfree(dev->host_mem_desc_bufs);
@@ -1815,8 +1816,9 @@ static int __nvme_alloc_host_mem(struct nvme_dev *dev,
u64 preferred,
while (--i >= 0) {
size_t size = le32_to_cpu(descs[i].size) * dev->ctrl.page_size;
- dma_free_coherent(dev->dev, size, bufs[i],
- le64_to_cpu(descs[i].addr));
+ dma_free_attrs(dev->dev, size, bufs[i],
+ le64_to_cpu(descs[i].addr),
+ DMA_ATTR_NO_KERNEL_MAPPING | DMA_ATTR_NO_WARN);
}
kfree(bufs);
--
2.20.1
Re: [PATCH v8 20/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte()
Finn Thain a écrit :
Make use of arch_nvram_ops in device drivers so that the nvram_* function
exports can be removed.
Since they are no longer global symbols, rename the PPC32 nvram_* functions
appropriately.
Signed-off-by: Finn Thain
---
arch/powerpc/kernel/setup_32.c | 8
drivers/char/generic_nvram.c | 4 ++--
drivers/video/fbdev/controlfb.c| 4 ++--
drivers/video/fbdev/imsttfb.c | 4 ++--
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
drivers/video/fbdev/platinumfb.c | 4 ++--
drivers/video/fbdev/valkyriefb.c | 4 ++--
7 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index e0d045677472..bdbe6acbef11 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -152,20 +152,18 @@ __setup("l3cr=", ppc_setup_l3cr);
#ifdef CONFIG_GENERIC_NVRAM
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
{
if (ppc_md.nvram_read_val)
return ppc_md.nvram_read_val(addr);
return 0xff;
}
-EXPORT_SYMBOL(nvram_read_byte);
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
{
if (ppc_md.nvram_write_val)
ppc_md.nvram_write_val(addr, val);
}
-EXPORT_SYMBOL(nvram_write_byte);
static ssize_t ppc_nvram_get_size(void)
{
@@ -182,6 +180,8 @@ static long ppc_nvram_sync(void)
}
const struct nvram_ops arch_nvram_ops = {
+ .read_byte = ppc_nvram_read_byte,
+ .write_byte = ppc_nvram_write_byte,
.get_size = ppc_nvram_get_size,
.sync = ppc_nvram_sync,
};
diff --git a/drivers/char/generic_nvram.c b/drivers/char/generic_nvram.c
index f32d5663de95..41b76bf9614e 100644
--- a/drivers/char/generic_nvram.c
+++ b/drivers/char/generic_nvram.c
@@ -48,7 +48,7 @@ static ssize_t read_nvram(struct file *file, char
__user *buf,
if (*ppos >= nvram_len)
return 0;
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
- if (__put_user(nvram_read_byte(i), p))
+ if (__put_user(arch_nvram_ops.read_byte(i), p))
Instead of modifying all drivers (in this patch and previous ones
related to other arches), wouldn't it be better to add helpers like
the following in nvram.h:
Static inline unsigned char nvram_read_byte(int addr)
{
return arch_nvram_ops.read_byte(addr);
}
Christophe
return -EFAULT;
*ppos = i;
return p - buf;
@@ -68,7 +68,7 @@ static ssize_t write_nvram(struct file *file,
const char __user *buf,
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
if (__get_user(c, p))
return -EFAULT;
- nvram_write_byte(c, i);
+ arch_nvram_ops.write_byte(c, i);
}
*ppos = i;
return p - buf;
diff --git a/drivers/video/fbdev/controlfb.c
b/drivers/video/fbdev/controlfb.c
index 9cb0ef7ac29e..27ff33ccafcb 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -413,7 +413,7 @@ static int __init init_control(struct fb_info_control *p)
/* Try to pick a video mode out of NVRAM if we have one. */
#ifdef CONFIG_NVRAM
if (default_cmode == CMODE_NVRAM) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if(cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
} else
@@ -421,7 +421,7 @@ static int __init init_control(struct fb_info_control *p)
cmode=default_cmode;
#ifdef CONFIG_NVRAM
if (default_vmode == VMODE_NVRAM) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode < 1 || vmode > VMODE_MAX ||
control_mac_modes[vmode - 1].m[full] < cmode) {
sense = read_control_sense(p);
diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c
index 8d231591ff0e..e9f3b8914145 100644
--- a/drivers/video/fbdev/imsttfb.c
+++ b/drivers/video/fbdev/imsttfb.c
@@ -1393,12 +1393,12 @@ static void init_imstt(struct fb_info *info)
int vmode = init_vmode, cmode = init_cmode;
if (vmode == -1) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode <= 0 || vmode > VMODE_MAX)
vmode = VMODE_640_480_67;
}
if (cmode == -1) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if (cmode < CMODE_8 || cmode > CMODE_32)
