* Re: [GIT] Networking
From: Linus Torvalds @ 2018-10-24 13:40 UTC (permalink / raw)
To: andy.gross-QSEj5FYQhm4dnm+yROfE0A
Cc: kvalo-A+ZNKFmMK5xy9aJCnZT0Uw, Bjorn Andersson, Govind Singh,
David Miller, Andrew Morton, netdev-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
niklas.cassel-QSEj5FYQhm4dnm+yROfE0A,
david.brown-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <CAPBZ5Qen9ak4eFqdHEKNVoGBWUtohu_hwYxZ2cwmyAU=a8Mv9A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Oct 24, 2018 at 2:24 PM Andy Gross <andy.gross-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
> Yes this will conflict with Niklas's patch which is part of the 4.20
> pull requests. I would prefer that we revert Linus's and take Niklas's
> unless there is a compelling argument to have it fixed before -rc1.
I have no objection to just reverting my patch when I get the real fix.
I just don't want my tree to have warnings that I see, and that may
hide new warnings coming in when I do my next pull request..
Linus
^ permalink raw reply
* Re: KASAN: use-after-free Read in do_exit
From: Eric W. Biederman @ 2018-10-24 13:54 UTC (permalink / raw)
To: syzbot
Cc: akpm, linux-kernel, linux, mark.rutland, paulmck,
sudipm.mukherjee, syzkaller-bugs, netdev, Boris Pismenny,
Daniel Borkmann, Eric Dumazet, Ilya Lesokhin, Aviad Yehezkel
In-Reply-To: <000000000000f6512e0578f70944@google.com>
Adding netdev and some likely suspects as this appears to happen in
tls_sk_proto_close.
syzbot <syzbot+71a06dcf7e167c30ab9d@syzkaller.appspotmail.com> writes:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 58a022870787 Merge tag 'acpi-4.20-rc1' of git://git.kernel..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12a0f939400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=414a8e66ba2c6789
> dashboard link: https://syzkaller.appspot.com/bug?extid=71a06dcf7e167c30ab9d
> 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+71a06dcf7e167c30ab9d@syzkaller.appspotmail.com
>
> TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending cookies.
> Check SNMP counters.
> input: syz1 as /devices/virtual/input/input1084
> netlink: 20 bytes leftover after parsing attributes in process `syz-executor2'.
> kasan: CONFIG_KASAN_INLINE enabled
> ==================================================================
> BUG: KASAN: use-after-free in stack_not_used
> include/linux/sched/task_stack.h:101 [inline]
> BUG: KASAN: use-after-free in check_stack_usage kernel/exit.c:748 [inline]
> BUG: KASAN: use-after-free in do_exit+0x2196/0x26d0 kernel/exit.c:916
> Read of size 8 at addr ffff8801c2b90008 by task blkid/5862
>
> CPU: 1 PID: 5862 Comm: blkid Not tainted 4.19.0+ #299
> 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+0x1c4/0x2b6 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
> stack_not_used include/linux/sched/task_stack.h:101 [inline]
> check_stack_usage kernel/exit.c:748 [inline]
> do_exit+0x2196/0x26d0 kernel/exit.c:916
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] PREEMPT SMP KASAN
> CPU: 0 PID: 5797 Comm: syz-executor4 Not tainted 4.19.0+ #299
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google
> 01/01/2011
> RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
> RIP: 0010:compound_head include/linux/page-flags.h:142 [inline]
> RIP: 0010:put_page include/linux/mm.h:931 [inline]
> RIP: 0010:tls_sk_proto_close+0x419/0xbb0 net/tls/tls_main.c:277
> Code: 8d e8 fe ff ff 48 89 85 f0 fe ff ff e8 b0 55 db fa 48 8b 85 f0 fe ff ff 49
> 83 e5 fc 49 8d 7d 08 c6 00 00 48 89 f8 48 c1 e8 03 <42> 80 3c 20 00 0f 85 ad 06
> 00 00 48 8b 85 f0 fe ff ff 4d 8b 75 08
> RSP: 0018:ffff88018c2e6f70 EFLAGS: 00010202
> RAX: 0000000000000001 RBX: ffff88018566ae40 RCX: ffffffff86a36050
> RDX: 0000000000000000 RSI: ffffffff86a35e10 RDI: 0000000000000008
> RBP: ffff88018c2e70d0 R08: ffff880182d0c580 R09: 0000000000000006
> R10: 0000000000000000 R11: ffff880182d0c580 R12: dffffc0000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: ffff88018ba82740
> FS: 00007f2e39ea9700(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000001b32334000 CR3: 00000001ccbdb000 CR4: 00000000001426f0
> Call Trace:
> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:428
> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:457
> __sock_release+0xd7/0x250 net/socket.c:579
> sock_close+0x19/0x20 net/socket.c:1141
> __fput+0x385/0xa30 fs/file_table.c:278
> do_group_exit+0x177/0x440 kernel/exit.c:970
> ____fput+0x15/0x20 fs/file_table.c:309
> task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
> __do_sys_exit_group kernel/exit.c:981 [inline]
> __se_sys_exit_group kernel/exit.c:979 [inline]
> __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:979
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x1ad6/0x26d0 kernel/exit.c:867
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x7f564809d1e8
> Code: Bad RIP value.
> RSP: 002b:00007ffd77478cc8 EFLAGS: 00000246
> ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f564809d1e8
> RDX: 0000000000000002 RSI: 000000000000003c RDI: 0000000000000002
> RBP: 00007f5648372840 R08: 00000000000000e7 R09: ffffffffffffffa8
> R10: 00007f5648378740 R11: 0000000000000246 R12: 00007f5648372840
> R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000000
>
> Allocated by task 5279:
> 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
> kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
> kmem_cache_alloc+0x12e/0x730 mm/slab.c:3554
> kmem_cache_zalloc include/linux/slab.h:697 [inline]
> locks_alloc_lock+0x8e/0x270 fs/locks.c:303
> fcntl_setlk+0xc8/0xfb0 fs/locks.c:2246
> do_fcntl+0xa6f/0x1500 fs/fcntl.c:370
> __do_sys_fcntl fs/fcntl.c:463 [inline]
> __se_sys_fcntl fs/fcntl.c:448 [inline]
> __x64_sys_fcntl+0x177/0x1f0 fs/fcntl.c:448
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 5279:
> 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]
> kmem_cache_free+0x83/0x290 mm/slab.c:3756
> locks_free_lock+0x209/0x340 fs/locks.c:339
> fcntl_setlk+0x79f/0xfb0 fs/locks.c:2323
> do_fcntl+0xa6f/0x1500 fs/fcntl.c:370
> __do_sys_fcntl fs/fcntl.c:463 [inline]
> __se_sys_fcntl fs/fcntl.c:448 [inline]
> __x64_sys_fcntl+0x177/0x1f0 fs/fcntl.c:448
> 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 ffff8801c2b90000
> which belongs to the cache file_lock_cache of size 248
> The buggy address is located 8 bytes inside of
> 248-byte region [ffff8801c2b90000, ffff8801c2b900f8)
> The buggy address belongs to the page:
> do_group_exit+0x177/0x440 kernel/exit.c:970
> page:ffffea00070ae400 count:1 mapcount:0 mapping:ffff8801d9bf5b00 index:0x0
> get_signal+0x8b0/0x1980 kernel/signal.c:2513
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea000709e388 ffffea0006039548 ffff8801d9bf5b00
> raw: 0000000000000000 ffff8801c2b90000 000000010000000d 0000000000000000
> page dumped because: kasan: bad access detected
> do_signal+0x9c/0x21e0 arch/x86/kernel/signal.c:816
>
> Memory state around the buggy address:
> ffff8801c2b8ff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801c2b8ff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff8801c2b90000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ^
> ffff8801c2b90080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fc
> ffff8801c2b90100: fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>
> ---
> 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 syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with syzbot.
^ permalink raw reply
* Re: [PATCH] r8169: Add new device ID support
From: David Miller @ 2018-10-24 5:54 UTC (permalink / raw)
To: shawn.lin; +Cc: nic_swsd, netdev
In-Reply-To: <37d7eebe-5abc-2fe2-adae-562e424f8108@rock-chips.com>
From: Shawn Lin <shawn.lin@rock-chips.com>
Date: Wed, 24 Oct 2018 13:48:55 +0800
> Hi David,
>
> On 2018/10/24 10:19, David Miller wrote:
>> From: Shawn Lin <shawn.lin@rock-chips.com>
>> Date: Wed, 24 Oct 2018 09:46:47 +0800
>>
>>> It's found my r8169 ethernet card at hand has a device ID
>>> of 0x0000 which wasn't on the list of rtl8169_pci_tbl. Add
>>> a new entry to make it work:
>> ...
>>> 01:00.0 Class 0200: 10ec:0000
>> I don't know about this.
>> A value of zero could mean the device is mis-responding to
>> PCI config space requests or something like that.
>
> It was working fine on my retired Windows XP home PC with same devcice
> ID listed, so I guess r8169 driver for windows system knows 0x0000 is
> also valid.
It is also possible the device comes up in a different state.
Under windows does it show with that device ID of zero?
^ permalink raw reply
* Re: [PATCH] r8169: Add new device ID support
From: Shawn Lin @ 2018-10-24 5:48 UTC (permalink / raw)
To: David Miller; +Cc: shawn.lin, nic_swsd, netdev
In-Reply-To: <20181023.191914.36695990470437174.davem@davemloft.net>
Hi David,
On 2018/10/24 10:19, David Miller wrote:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> Date: Wed, 24 Oct 2018 09:46:47 +0800
>
>> It's found my r8169 ethernet card at hand has a device ID
>> of 0x0000 which wasn't on the list of rtl8169_pci_tbl. Add
>> a new entry to make it work:
> ...
>> 01:00.0 Class 0200: 10ec:0000
>
> I don't know about this.
>
> A value of zero could mean the device is mis-responding to
> PCI config space requests or something like that.
It was working fine on my retired Windows XP home PC with same devcice
ID listed, so I guess r8169 driver for windows system knows 0x0000 is
also valid.
>
>
>
^ permalink raw reply
* [PATCH net-next] net: hns3: Fix for warning uninitialized symbol hw_err_lst3
From: Salil Mehta @ 2018-10-24 14:32 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, dan.carpenter,
netdev, linux-kernel, linuxarm, Shiju Jose
From: Shiju Jose <shiju.jose@huawei.com>
This patch fixes the smatch warning,
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c:700
hclge_log_and_clear_ppp_error() error: uninitialized symbol
'hw_err_lst3'
Link: https://lkml.org/lkml/2018/10/23/430
Fixes: da2d072a9ea7 ("net: hns3: Add enable and process hw errors from PPP")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
index f7e363b..dca6f23 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_err.c
@@ -859,10 +859,12 @@ static int hclge_log_and_clear_ppp_error(struct hclge_dev *hdev, u32 cmd,
reset_level = HNAE3_FUNC_RESET;
}
- err_sts = (le32_to_cpu(desc[0].data[4]) >> 8) & 0x3;
- if (err_sts) {
- hclge_log_error(dev, hw_err_lst3, err_sts);
- reset_level = HNAE3_FUNC_RESET;
+ if (cmd == HCLGE_PPP_CMD0_INT_CMD) {
+ err_sts = (le32_to_cpu(desc[0].data[4]) >> 8) & 0x3;
+ if (err_sts) {
+ hclge_log_error(dev, hw_err_lst3, err_sts);
+ reset_level = HNAE3_FUNC_RESET;
+ }
}
/* clear PPP INT */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] r8169: Add new device ID support
From: Shawn Lin @ 2018-10-24 6:29 UTC (permalink / raw)
To: David Miller; +Cc: shawn.lin, nic_swsd, netdev
In-Reply-To: <20181023.225442.1195260610454764659.davem@davemloft.net>
On 2018/10/24 13:54, David Miller wrote:
> From: Shawn Lin <shawn.lin@rock-chips.com>
> Date: Wed, 24 Oct 2018 13:48:55 +0800
>
>> Hi David,
>>
>> On 2018/10/24 10:19, David Miller wrote:
>>> From: Shawn Lin <shawn.lin@rock-chips.com>
>>> Date: Wed, 24 Oct 2018 09:46:47 +0800
>>>
>>>> It's found my r8169 ethernet card at hand has a device ID
>>>> of 0x0000 which wasn't on the list of rtl8169_pci_tbl. Add
>>>> a new entry to make it work:
>>> ...
>>>> 01:00.0 Class 0200: 10ec:0000
>>> I don't know about this.
>>> A value of zero could mean the device is mis-responding to
>>> PCI config space requests or something like that.
>>
>> It was working fine on my retired Windows XP home PC with same devcice
>> ID listed, so I guess r8169 driver for windows system knows 0x0000 is
>> also valid.
>
> It is also possible the device comes up in a different state.
>
> Under windows does it show with that device ID of zero?
yup. More precisely, I checked how BIOS enumerate it by PCIe analyzer
and see it does report 0x0000 as device ID.
>
>
>
^ permalink raw reply
* Re: [PATCH ghak90 (was ghak32) V4 03/10] audit: log container info of syscalls
From: Richard Guy Briggs @ 2018-10-24 15:14 UTC (permalink / raw)
To: Paul Moore
Cc: containers, linux-api, linux-audit, linux-fsdevel, linux-kernel,
netdev, netfilter-devel, ebiederm, luto, carlos, dhowells, viro,
simo, Eric Paris, Serge Hallyn
In-Reply-To: <CAHC9VhSv4hWGHiaaOfuBrRQKdp2YG5RtqXQbFXg7j7LNEhO2XQ@mail.gmail.com>
On 2018-10-19 19:16, Paul Moore wrote:
> On Sun, Aug 5, 2018 at 4:32 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > Create a new audit record AUDIT_CONTAINER to document the audit
> > container identifier of a process if it is present.
> >
> > Called from audit_log_exit(), syscalls are covered.
> >
> > A sample raw event:
> > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257 success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2 ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="tmpcontainerid"
> > type=CWD msg=audit(1519924845.499:257): cwd="/root"
> > type=PATH msg=audit(1519924845.499:257): item=0 name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PATH msg=audit(1519924845.499:257): item=1 name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PROCTITLE msg=audit(1519924845.499:257): proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > type=CONTAINER msg=audit(1519924845.499:257): op=task contid=123458
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/90
> > See: https://github.com/linux-audit/audit-userspace/issues/51
> > See: https://github.com/linux-audit/audit-testsuite/issues/64
> > See: https://github.com/linux-audit/audit-kernel/wiki/RFE-Audit-Container-ID
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> > Acked-by: Steve Grubb <sgrubb@redhat.com>
> > ---
> > include/linux/audit.h | 7 +++++++
> > include/uapi/linux/audit.h | 1 +
> > kernel/audit.c | 24 ++++++++++++++++++++++++
> > kernel/auditsc.c | 3 +++
> > 4 files changed, 35 insertions(+)
>
> ...
>
> > @@ -2045,6 +2045,30 @@ void audit_log_session_info(struct audit_buffer *ab)
> > audit_log_format(ab, " auid=%u ses=%u", auid, sessionid);
> > }
> >
> > +/*
> > + * audit_log_contid - report container info
> > + * @tsk: task to be recorded
> > + * @context: task or local context for record
> > + * @op: contid string description
> > + */
> > +int audit_log_contid(struct task_struct *tsk,
> > + struct audit_context *context, char *op)
> > +{
> > + struct audit_buffer *ab;
> > +
> > + if (!audit_contid_set(tsk))
> > + return 0;
> > + /* Generate AUDIT_CONTAINER record with container ID */
> > + ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONTAINER);
> > + if (!ab)
> > + return -ENOMEM;
> > + audit_log_format(ab, "op=%s contid=%llu",
> > + op, audit_get_contid(tsk));
> > + audit_log_end(ab);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(audit_log_contid);
>
> As discussed in the previous iteration of the patch, I prefer
> AUDIT_CONTAINER_ID here over AUDIT_CONTAINER. If you feel strongly
> about keeping it as-is with AUDIT_CONTAINER I suppose I could live
> with that, but it is isn't my first choice.
I don't have a strong opinion on this one, mildly preferring the shorter
one only because it is shorter.
Steve? Can you comment on this one way or the other?
> However, I do care about the "op" field in this record. It just
> doesn't make any sense; the way you are using it it is more of a
> context field than an operations field, and even then why is the
> context important from a logging and/or security perspective? Drop it
> please.
I'll rename it to whatever you like. I'd suggest "ref=". The reason I
think it is important is there are multiple sources that aren't always
obvious from the other records to which it is associated. In the case
of ptrace and signals, there can be many target tasks listed (OBJ_PID)
with no other way to distinguish the matching audit container identifier
records all for one event. This is in addition to the default syscall
container identifier record. I'm not currently happy with the text
content to link the two, but that should be solvable (most obvious is
taret PID). Throwing away this information seems shortsighted.
> paul moore
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Willy Tarreau @ 2018-10-24 15:57 UTC (permalink / raw)
To: Wang Hai; +Cc: edumazet, davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20181024154729.5312-1-wanghaifine@gmail.com>
On Wed, Oct 24, 2018 at 11:47:29PM +0800, Wang Hai wrote:
> To determine whether len is less than zero, it should be put before
> the function min_t, because the return value of min_t is not likely
> to be less than zero.
Huh? First, the <0 test is made on "len", not "min_t", so it still
is signed. Second, you're in fact completely removing the test here,
look :
> struct net *net = sock_net(sk);
> int val, len;
>
> + len = min_t(unsigned int, len, sizeof(int));
> +
len is used uninitialized here, so the result is undefined.
> if (get_user(len, optlen))
> return -EFAULT;
Then it gets overridden by get_user()
> - len = min_t(unsigned int, len, sizeof(int));
> -
Then its positive values are not bounded anymore since you moved the test.
> if (len < 0)
> return -EINVAL;
Then only negative values are dropped. So unless I'm missing something
obvious, you're just allowing len to be as large as 2GB-1 based on the
user's fed optlen.
Am I wrong ?
Willy
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Eric Dumazet @ 2018-10-24 15:59 UTC (permalink / raw)
To: wanghaifine
Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, LKML
In-Reply-To: <20181024154729.5312-1-wanghaifine@gmail.com>
On Wed, Oct 24, 2018 at 8:47 AM Wang Hai <wanghaifine@gmail.com> wrote:
>
> To determine whether len is less than zero, it should be put before
> the function min_t, because the return value of min_t is not likely
> to be less than zero.
????
I really wonder if you actually tested this patch.
Thanks.
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Willy Tarreau @ 2018-10-24 16:01 UTC (permalink / raw)
To: Eric Dumazet
Cc: wanghaifine, David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
netdev, LKML
In-Reply-To: <CANn89iJ__4gXH-6ns5fiun6G29ppYWLRE_G=4s0vTus+ccGXcQ@mail.gmail.com>
On Wed, Oct 24, 2018 at 08:59:39AM -0700, Eric Dumazet wrote:
> I really wonder if you actually tested this patch.
I'm even wondering if it's not done on purpose to retrieve up to 2 GB of
kernel memory via a single getsockopt() call :-/
Willy
^ permalink raw reply
* [PATCH v3 0/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64
From: Corentin Labbe @ 2018-10-24 7:35 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
matthias.bgg, maxime.ripard, michal.lkml, mpe, mporter,
narmstrong, nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb,
wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linux-mediatek, linuxppc-dev, netdev, linux-sunxi,
Corentin Labbe
Hello
This patchset adds a new set of functions which are open-coded in lot of
place.
Basicly the pattern is always the same, "read, modify a bit, write"
some driver and the powerpc arch already have thoses pattern them as functions. (like ahci_sunxi.c or dwmac-meson8b)
The first patch rename some powerpc functions for being consistent with
the new name convention.
The second patch adds the header with all setbits functions.
The third patch is a try to implement a coccinelle semantic patch to
find all place where xxxbits function could be used.
It should not be merged since it is un-finalized.
For the moment, the "add setbits.h header" is not working and need a future
coccinelle version.
The four last patch are example of some drivers conversion.
Thoses patchs give an example of the reduction of code won by using xxxbits32.
I would like to thanks Julia Lawall for her help on the coccinelle
patch.
Note that I dont know which maintainer will take the linux/setbits.h include patch.
Regards
Changes since v2:
- Fixed patch title
- Fixed style problems
- shorted macro arguments name
Changes since v1:
- renamed LE functions to _leXX
- updated coccinnelle patch with JLawall's comments
Corentin Labbe (7):
powerpc: rename setbits32/clrbits32 to setbits_be32/clrbits_be32
include: add setbits_leXX/clrbits_leXX/clrsetbits_leXX in
linux/setbits.h
coccinelle: add xxxsetbits_leXX converting spatch
ata: ahci_sunxi: use xxxsetbitsi_le32 functions
net: ethernet: stmmac: dwmac-sun8i: use xxxsetbits_le32
drm: meson: use xxxsetbits_le32
net: stmmac: dwmac-meson8b: use xxxsetbits_le32
arch/powerpc/include/asm/fsl_lbc.h | 2 +-
arch/powerpc/include/asm/io.h | 4 +-
arch/powerpc/platforms/44x/canyonlands.c | 4 +-
arch/powerpc/platforms/4xx/gpio.c | 28 +-
arch/powerpc/platforms/512x/pdm360ng.c | 6 +-
arch/powerpc/platforms/52xx/mpc52xx_common.c | 6 +-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 12 +-
arch/powerpc/platforms/82xx/ep8248e.c | 2 +-
arch/powerpc/platforms/82xx/km82xx.c | 6 +-
arch/powerpc/platforms/82xx/mpc8272_ads.c | 10 +-
arch/powerpc/platforms/82xx/pq2.c | 2 +-
arch/powerpc/platforms/82xx/pq2ads-pci-pic.c | 4 +-
arch/powerpc/platforms/82xx/pq2fads.c | 10 +-
arch/powerpc/platforms/83xx/km83xx.c | 6 +-
arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 +-
arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 2 +-
arch/powerpc/platforms/85xx/p1022_ds.c | 6 +-
arch/powerpc/platforms/85xx/p1022_rdk.c | 6 +-
arch/powerpc/platforms/85xx/t1042rdb_diu.c | 6 +-
arch/powerpc/platforms/85xx/twr_p102x.c | 2 +-
arch/powerpc/platforms/86xx/mpc8610_hpcd.c | 6 +-
arch/powerpc/platforms/8xx/adder875.c | 2 +-
arch/powerpc/platforms/8xx/m8xx_setup.c | 4 +-
arch/powerpc/platforms/8xx/mpc86xads_setup.c | 4 +-
arch/powerpc/platforms/8xx/mpc885ads_setup.c | 28 +-
.../platforms/embedded6xx/flipper-pic.c | 6 +-
arch/powerpc/platforms/embedded6xx/hlwd-pic.c | 8 +-
arch/powerpc/platforms/embedded6xx/wii.c | 12 +-
arch/powerpc/sysdev/cpm1.c | 26 +-
arch/powerpc/sysdev/cpm2.c | 16 +-
arch/powerpc/sysdev/cpm_common.c | 4 +-
arch/powerpc/sysdev/fsl_85xx_l2ctlr.c | 16 +-
arch/powerpc/sysdev/fsl_lbc.c | 2 +-
arch/powerpc/sysdev/fsl_pci.c | 12 +-
arch/powerpc/sysdev/fsl_pmc.c | 2 +-
arch/powerpc/sysdev/fsl_rcpm.c | 74 +--
arch/powerpc/sysdev/fsl_rio.c | 4 +-
arch/powerpc/sysdev/fsl_rmu.c | 9 +-
arch/powerpc/sysdev/mpic_timer.c | 12 +-
drivers/ata/ahci_sunxi.c | 62 +--
drivers/gpu/drm/meson/meson_crtc.c | 14 +-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 33 +-
drivers/gpu/drm/meson/meson_plane.c | 13 +-
drivers/gpu/drm/meson/meson_registers.h | 3 -
drivers/gpu/drm/meson/meson_venc.c | 13 +-
drivers/gpu/drm/meson/meson_venc_cvbs.c | 4 +-
drivers/gpu/drm/meson/meson_viu.c | 65 +--
drivers/gpu/drm/meson/meson_vpp.c | 22 +-
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 +-
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 62 +--
include/linux/setbits.h | 84 +++
scripts/add_new_include_in_source.py | 61 +++
scripts/coccinelle/misc/setbits32.cocci | 487 ++++++++++++++++++
.../coccinelle/misc/setbits32_relaxed.cocci | 487 ++++++++++++++++++
scripts/coccinelle/misc/setbits64.cocci | 487 ++++++++++++++++++
scripts/coccinelle/misc/setbits_dev.cocci | 235 +++++++++
58 files changed, 2172 insertions(+), 395 deletions(-)
create mode 100644 include/linux/setbits.h
create mode 100755 scripts/add_new_include_in_source.py
create mode 100644 scripts/coccinelle/misc/setbits32.cocci
create mode 100644 scripts/coccinelle/misc/setbits32_relaxed.cocci
create mode 100644 scripts/coccinelle/misc/setbits64.cocci
create mode 100644 scripts/coccinelle/misc/setbits_dev.cocci
--
2.18.1
^ permalink raw reply
* [PATCH v3 2/7] include: add setbits_leXX/clrbits_leXX/clrsetbits_leXX in linux/setbits.h
From: Corentin Labbe @ 2018-10-24 7:35 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
matthias.bgg, maxime.ripard, michal.lkml, mpe, mporter,
narmstrong, nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb,
wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linux-mediatek, linuxppc-dev, netdev, linux-sunxi,
Corentin Labbe
In-Reply-To: <1540366553-18541-1-git-send-email-clabbe@baylibre.com>
This patch adds setbits_le32/clrbits_le32/clrsetbits_le32 and
setbits_le64/clrbits_le64/clrsetbits_le64 in linux/setbits.h header.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
include/linux/setbits.h | 84 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 include/linux/setbits.h
diff --git a/include/linux/setbits.h b/include/linux/setbits.h
new file mode 100644
index 000000000000..c82faf8d7fe4
--- /dev/null
+++ b/include/linux/setbits.h
@@ -0,0 +1,84 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_SETBITS_H
+#define __LINUX_SETBITS_H
+
+#include <linux/io.h>
+
+#define __setbits(rfn, wfn, addr, set) wfn((rfn(addr) | (set)), addr)
+#define __clrbits(rfn, wfn, addr, mask) wfn((rfn(addr) & ~(mask)), addr)
+#define __clrsetbits(rfn, wfn, addr, mask, set) wfn(((rfn(addr) & ~(mask)) | (set)), addr)
+#define __setclrbits(rfn, wfn, addr, mask, set) wfn(((rfn(addr) | (set)) & ~(mask)), addr)
+
+#ifndef setbits_le32
+#define setbits_le32(addr, set) __setbits(readl, writel, addr, set)
+#endif
+#ifndef setbits_le32_relaxed
+#define setbits_le32_relaxed(addr, set) __setbits(readl_relaxed, writel_relaxed, \
+ addr, set)
+#endif
+
+#ifndef clrbits_le32
+#define clrbits_le32(addr, mask) __clrbits(readl, writel, addr, mask)
+#endif
+#ifndef clrbits_le32_relaxed
+#define clrbits_le32_relaxed(addr, mask) __clrbits(readl_relaxed, writel_relaxed, \
+ addr, mask)
+#endif
+
+#ifndef clrsetbits_le32
+#define clrsetbits_le32(addr, mask, set) __clrsetbits(readl, writel, addr, mask, set)
+#endif
+#ifndef clrsetbits_le32_relaxed
+#define clrsetbits_le32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
+ writel_relaxed, \
+ addr, mask, set)
+#endif
+
+#ifndef setclrbits_le32
+#define setclrbits_le32(addr, mask, set) __setclrbits(readl, writel, addr, mask, set)
+#endif
+#ifndef setclrbits_le32_relaxed
+#define setclrbits_le32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
+ writel_relaxed, \
+ addr, mask, set)
+#endif
+
+/* We cannot use CONFIG_64BIT as some x86 drivers use non-atomicwriteq() */
+#if defined(writeq) && defined(readq)
+#ifndef setbits_le64
+#define setbits_le64(addr, set) __setbits(readq, writeq, addr, set)
+#endif
+#ifndef setbits_le64_relaxed
+#define setbits_le64_relaxed(addr, set) __setbits(readq_relaxed, writeq_relaxed, \
+ addr, set)
+#endif
+
+#ifndef clrbits_le64
+#define clrbits_le64(addr, mask) __clrbits(readq, writeq, addr, mask)
+#endif
+#ifndef clrbits_le64_relaxed
+#define clrbits_le64_relaxed(addr, mask) __clrbits(readq_relaxed, writeq_relaxed, \
+ addr, mask)
+#endif
+
+#ifndef clrsetbits_le64
+#define clrsetbits_le64(addr, mask, set) __clrsetbits(readq, writeq, addr, mask, set)
+#endif
+#ifndef clrsetbits_le64_relaxed
+#define clrsetbits_le64_relaxed(addr, mask, set) __clrsetbits(readq_relaxed, \
+ writeq_relaxed, \
+ addr, mask, set)
+#endif
+
+#ifndef setclrbits_le64
+#define setclrbits_le64(addr, mask, set) __setclrbits(readq, writeq, addr, mask, set)
+#endif
+#ifndef setclrbits_le64_relaxed
+#define setclrbits_le64_relaxed(addr, mask, set) __setclrbits(readq_relaxed, \
+ writeq_relaxed, \
+ addr, mask, set)
+#endif
+
+#endif /* writeq/readq */
+
+#endif /* __LINUX_SETBITS_H */
--
2.18.1
^ permalink raw reply related
* [PATCH v3 6/7] drm: meson: use xxxsetbits_le32
From: Corentin Labbe @ 2018-10-24 7:35 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
matthias.bgg, maxime.ripard, michal.lkml, mpe, mporter,
narmstrong, nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb,
wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linux-mediatek, linuxppc-dev, netdev, linux-sunxi,
Corentin Labbe
In-Reply-To: <1540366553-18541-1-git-send-email-clabbe@baylibre.com>
This patch convert meson DRM driver to use all xxxsetbits_le32 functions.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Tested-by: Neil Armstrong <narmstrong@baylibre.com>
---
drivers/gpu/drm/meson/meson_crtc.c | 14 +++---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 33 +++++++------
drivers/gpu/drm/meson/meson_plane.c | 13 ++---
drivers/gpu/drm/meson/meson_registers.h | 3 --
drivers/gpu/drm/meson/meson_venc.c | 13 ++---
drivers/gpu/drm/meson/meson_venc_cvbs.c | 4 +-
drivers/gpu/drm/meson/meson_viu.c | 65 +++++++++++++------------
drivers/gpu/drm/meson/meson_vpp.c | 22 ++++-----
8 files changed, 86 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
index 05520202c967..98f17ddd6b00 100644
--- a/drivers/gpu/drm/meson/meson_crtc.c
+++ b/drivers/gpu/drm/meson/meson_crtc.c
@@ -25,6 +25,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
@@ -98,8 +99,8 @@ static void meson_crtc_atomic_enable(struct drm_crtc *crtc,
writel(crtc_state->mode.hdisplay,
priv->io_base + _REG(VPP_POSTBLEND_H_SIZE));
- writel_bits_relaxed(VPP_POSTBLEND_ENABLE, VPP_POSTBLEND_ENABLE,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_POSTBLEND_ENABLE, VPP_POSTBLEND_ENABLE);
priv->viu.osd1_enabled = true;
}
@@ -114,8 +115,8 @@ static void meson_crtc_atomic_disable(struct drm_crtc *crtc,
priv->viu.osd1_commit = false;
/* Disable VPP Postblend */
- writel_bits_relaxed(VPP_POSTBLEND_ENABLE, 0,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_POSTBLEND_ENABLE, 0);
if (crtc->state->event && !crtc->state->active) {
spin_lock_irq(&crtc->dev->event_lock);
@@ -199,8 +200,9 @@ void meson_crtc_irq(struct meson_drm *priv)
MESON_CANVAS_BLKMODE_LINEAR);
/* Enable OSD1 */
- writel_bits_relaxed(VPP_OSD1_POSTBLEND, VPP_OSD1_POSTBLEND,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_OSD1_POSTBLEND,
+ VPP_OSD1_POSTBLEND);
priv->viu.osd1_commit = false;
}
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index df7247cd93f9..99a136209e15 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -24,6 +24,7 @@
#include <linux/reset.h>
#include <linux/clk.h>
#include <linux/regulator/consumer.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include <drm/drm_edid.h>
@@ -427,10 +428,10 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
/* Temporary Disable HDMI video stream to HDMI-TX */
- writel_bits_relaxed(0x3, 0,
- priv->io_base + _REG(VPU_HDMI_SETTING));
- writel_bits_relaxed(0xf << 8, 0,
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING), 0x3,
+ 0);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING),
+ 0xf << 8, 0);
/* Re-Enable VENC video stream */
if (priv->venc.hdmi_use_enci)
@@ -439,16 +440,16 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
/* Push back HDMI clock settings */
- writel_bits_relaxed(0xf << 8, wr_clk & (0xf << 8),
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING),
+ 0xf << 8, wr_clk & (0xf << 8));
/* Enable and Select HDMI video source for HDMI-TX */
if (priv->venc.hdmi_use_enci)
- writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCI,
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING),
+ 0x3, MESON_VENC_SOURCE_ENCI);
else
- writel_bits_relaxed(0x3, MESON_VENC_SOURCE_ENCP,
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING),
+ 0x3, MESON_VENC_SOURCE_ENCP);
return 0;
}
@@ -632,8 +633,8 @@ static void meson_venc_hdmi_encoder_disable(struct drm_encoder *encoder)
DRM_DEBUG_DRIVER("\n");
- writel_bits_relaxed(0x3, 0,
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING), 0x3,
+ 0);
writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
@@ -857,10 +858,10 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
reset_control_reset(meson_dw_hdmi->hdmitx_phy);
/* Enable APB3 fail on error */
- writel_bits_relaxed(BIT(15), BIT(15),
- meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG);
- writel_bits_relaxed(BIT(15), BIT(15),
- meson_dw_hdmi->hdmitx + HDMITX_DWC_CTRL_REG);
+ clrsetbits_le32_relaxed(meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG,
+ BIT(15), BIT(15));
+ clrsetbits_le32_relaxed(meson_dw_hdmi->hdmitx + HDMITX_DWC_CTRL_REG,
+ BIT(15), BIT(15));
/* Bring out of reset */
dw_hdmi_top_write(meson_dw_hdmi, HDMITX_TOP_SW_RESET, 0);
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index 12c80dfcff59..7377aefcbb2a 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -25,6 +25,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/platform_device.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include <drm/drm_atomic.h>
#include <drm/drm_atomic_helper.h>
@@ -115,15 +116,15 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
switch (fb->format->format) {
case DRM_FORMAT_XRGB8888:
/* For XRGB, replace the pixel's alpha by 0xFF */
- writel_bits_relaxed(OSD_REPLACE_EN, OSD_REPLACE_EN,
- priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_CTRL_STAT2),
+ OSD_REPLACE_EN, OSD_REPLACE_EN);
priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
OSD_COLOR_MATRIX_32_ARGB;
break;
case DRM_FORMAT_ARGB8888:
/* For ARGB, use the pixel's alpha */
- writel_bits_relaxed(OSD_REPLACE_EN, 0,
- priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_CTRL_STAT2),
+ OSD_REPLACE_EN, 0);
priv->viu.osd1_blk0_cfg[0] |= OSD_BLK_MODE_32 |
OSD_COLOR_MATRIX_32_ARGB;
break;
@@ -174,8 +175,8 @@ static void meson_plane_atomic_disable(struct drm_plane *plane,
struct meson_drm *priv = meson_plane->priv;
/* Disable OSD1 */
- writel_bits_relaxed(VPP_OSD1_POSTBLEND, 0,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_OSD1_POSTBLEND, 0);
}
diff --git a/drivers/gpu/drm/meson/meson_registers.h b/drivers/gpu/drm/meson/meson_registers.h
index bca87143e548..03ff452655f2 100644
--- a/drivers/gpu/drm/meson/meson_registers.h
+++ b/drivers/gpu/drm/meson/meson_registers.h
@@ -19,9 +19,6 @@
/* Shift all registers by 2 */
#define _REG(reg) ((reg) << 2)
-#define writel_bits_relaxed(mask, val, addr) \
- writel_relaxed((readl_relaxed(addr) & ~(mask)) | (val), addr)
-
/* vpp2 */
#define VPP2_DUMMY_DATA 0x1900
#define VPP2_LINE_IN_LENGTH 0x1901
diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
index 514245e69b38..eeb59a51f316 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -19,6 +19,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include "meson_drv.h"
#include "meson_venc.h"
@@ -913,8 +914,8 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
hsync_pixels_venc *= 2;
/* Disable VDACs */
- writel_bits_relaxed(0xff, 0xff,
- priv->io_base + _REG(VENC_VDAC_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VENC_VDAC_SETTING), 0xff,
+ 0xff);
writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
@@ -1250,8 +1251,8 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv, int vic,
writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
/* Set DE signal’s polarity is active high */
- writel_bits_relaxed(BIT(14), BIT(14),
- priv->io_base + _REG(ENCP_VIDEO_MODE));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ENCP_VIDEO_MODE),
+ BIT(14), BIT(14));
/* Program DE timing */
de_h_begin = modulo(readl_relaxed(priv->io_base +
@@ -1549,8 +1550,8 @@ void meson_venc_init(struct meson_drm *priv)
regmap_write(priv->hhi, HHI_HDMI_PHY_CNTL0, 0);
/* Disable HDMI */
- writel_bits_relaxed(0x3, 0,
- priv->io_base + _REG(VPU_HDMI_SETTING));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING), 0x3,
+ 0);
/* Disable all encoders */
writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
index f7945bae3b4a..6fff94d69e85 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
@@ -24,6 +24,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of_graph.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include <drm/drm_edid.h>
@@ -177,7 +178,8 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
struct meson_drm *priv = meson_venc_cvbs->priv;
/* VDAC0 source is not from ATV */
- writel_bits_relaxed(BIT(5), 0, priv->io_base + _REG(VENC_VDAC_DACSEL0));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VENC_VDAC_DACSEL0),
+ BIT(5), 0);
if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c
index 6bcfa527c180..952b74e874af 100644
--- a/drivers/gpu/drm/meson/meson_viu.c
+++ b/drivers/gpu/drm/meson/meson_viu.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include "meson_drv.h"
#include "meson_viu.h"
@@ -131,16 +132,16 @@ void meson_viu_set_osd_matrix(struct meson_drm *priv,
writel(m[20] & 0xfff,
priv->io_base + _REG(VIU_OSD1_MATRIX_OFFSET2));
- writel_bits_relaxed(3 << 30, m[21] << 30,
- priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
- writel_bits_relaxed(7 << 16, m[22] << 16,
- priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42),
+ 3 << 30, m[21] << 30);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_MATRIX_COLMOD_COEF42),
+ 7 << 16, m[22] << 16);
/* 23 reserved for clipping control */
- writel_bits_relaxed(BIT(0), csc_on ? BIT(0) : 0,
- priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
- writel_bits_relaxed(BIT(1), 0,
- priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL),
+ BIT(0), csc_on ? BIT(0) : 0);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_MATRIX_CTRL),
+ BIT(1), 0);
} else if (m_select == VIU_MATRIX_OSD_EOTF) {
int i;
@@ -150,10 +151,10 @@ void meson_viu_set_osd_matrix(struct meson_drm *priv,
(m[i * 2 + 1] & 0x1fff), priv->io_base +
_REG(VIU_OSD1_EOTF_CTL + i + 1));
- writel_bits_relaxed(BIT(30), csc_on ? BIT(30) : 0,
- priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
- writel_bits_relaxed(BIT(31), csc_on ? BIT(31) : 0,
- priv->io_base + _REG(VIU_OSD1_EOTF_CTL));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_EOTF_CTL),
+ BIT(30), csc_on ? BIT(30) : 0);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_EOTF_CTL),
+ BIT(31), csc_on ? BIT(31) : 0);
}
}
@@ -203,11 +204,11 @@ void meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e lut_sel,
priv->io_base + _REG(data_port));
if (csc_on)
- writel_bits_relaxed(0x7 << 29, 7 << 29,
- priv->io_base + _REG(ctrl_port));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ctrl_port),
+ 0x7 << 29, 7 << 29);
else
- writel_bits_relaxed(0x7 << 29, 0,
- priv->io_base + _REG(ctrl_port));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ctrl_port),
+ 0x7 << 29, 0);
} else if (lut_sel == VIU_LUT_OSD_EOTF) {
writel(0, priv->io_base + _REG(addr_port));
@@ -230,14 +231,14 @@ void meson_viu_set_osd_lut(struct meson_drm *priv, enum viu_lut_sel_e lut_sel,
priv->io_base + _REG(data_port));
if (csc_on)
- writel_bits_relaxed(7 << 27, 7 << 27,
- priv->io_base + _REG(ctrl_port));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ctrl_port),
+ 7 << 27, 7 << 27);
else
- writel_bits_relaxed(7 << 27, 0,
- priv->io_base + _REG(ctrl_port));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ctrl_port),
+ 7 << 27, 0);
- writel_bits_relaxed(BIT(31), BIT(31),
- priv->io_base + _REG(ctrl_port));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(ctrl_port),
+ BIT(31), BIT(31));
}
}
@@ -301,10 +302,10 @@ void meson_viu_init(struct meson_drm *priv)
uint32_t reg;
/* Disable OSDs */
- writel_bits_relaxed(BIT(0) | BIT(21), 0,
- priv->io_base + _REG(VIU_OSD1_CTRL_STAT));
- writel_bits_relaxed(BIT(0) | BIT(21), 0,
- priv->io_base + _REG(VIU_OSD2_CTRL_STAT));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_CTRL_STAT),
+ BIT(0) | BIT(21), 0);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD2_CTRL_STAT),
+ BIT(0) | BIT(21), 0);
/* On GXL/GXM, Use the 10bit HDR conversion matrix */
if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
@@ -322,12 +323,12 @@ void meson_viu_init(struct meson_drm *priv)
writel_relaxed(reg, priv->io_base + _REG(VIU_OSD2_FIFO_CTRL_STAT));
/* Set OSD alpha replace value */
- writel_bits_relaxed(0xff << OSD_REPLACE_SHIFT,
- 0xff << OSD_REPLACE_SHIFT,
- priv->io_base + _REG(VIU_OSD1_CTRL_STAT2));
- writel_bits_relaxed(0xff << OSD_REPLACE_SHIFT,
- 0xff << OSD_REPLACE_SHIFT,
- priv->io_base + _REG(VIU_OSD2_CTRL_STAT2));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD1_CTRL_STAT2),
+ 0xff << OSD_REPLACE_SHIFT,
+ 0xff << OSD_REPLACE_SHIFT);
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_OSD2_CTRL_STAT2),
+ 0xff << OSD_REPLACE_SHIFT,
+ 0xff << OSD_REPLACE_SHIFT);
priv->viu.osd1_enabled = false;
priv->viu.osd1_commit = false;
diff --git a/drivers/gpu/drm/meson/meson_vpp.c b/drivers/gpu/drm/meson/meson_vpp.c
index 27356f81a0ab..f36254485486 100644
--- a/drivers/gpu/drm/meson/meson_vpp.c
+++ b/drivers/gpu/drm/meson/meson_vpp.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/setbits.h>
#include <drm/drmP.h>
#include "meson_drv.h"
#include "meson_vpp.h"
@@ -128,30 +129,29 @@ void meson_vpp_init(struct meson_drm *priv)
if (meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
writel_relaxed(0x108080, priv->io_base + _REG(VPP_DUMMY_DATA1));
else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu")) {
- writel_bits_relaxed(0xff << 16, 0xff << 16,
- priv->io_base + _REG(VIU_MISC_CTRL1));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VIU_MISC_CTRL1),
+ 0xff << 16, 0xff << 16);
writel_relaxed(0x20000, priv->io_base + _REG(VPP_DOLBY_CTRL));
writel_relaxed(0x1020080,
priv->io_base + _REG(VPP_DUMMY_DATA1));
}
/* Initialize vpu fifo control registers */
- writel_relaxed(readl_relaxed(priv->io_base + _REG(VPP_OFIFO_SIZE)) |
- 0x77f, priv->io_base + _REG(VPP_OFIFO_SIZE));
+ setbits_le32_relaxed(priv->io_base + _REG(VPP_OFIFO_SIZE), 0x77f);
writel_relaxed(0x08080808, priv->io_base + _REG(VPP_HOLD_LINES));
/* Turn off preblend */
- writel_bits_relaxed(VPP_PREBLEND_ENABLE, 0,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_PREBLEND_ENABLE, 0);
/* Turn off POSTBLEND */
- writel_bits_relaxed(VPP_POSTBLEND_ENABLE, 0,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_POSTBLEND_ENABLE, 0);
/* Force all planes off */
- writel_bits_relaxed(VPP_OSD1_POSTBLEND | VPP_OSD2_POSTBLEND |
- VPP_VD1_POSTBLEND | VPP_VD2_POSTBLEND, 0,
- priv->io_base + _REG(VPP_MISC));
+ clrsetbits_le32_relaxed(priv->io_base + _REG(VPP_MISC),
+ VPP_OSD1_POSTBLEND | VPP_OSD2_POSTBLEND | VPP_VD1_POSTBLEND | VPP_VD2_POSTBLEND,
+ 0);
/* Disable Scalers */
writel_relaxed(0, priv->io_base + _REG(VPP_OSD_SC_CTRL0));
--
2.18.1
^ permalink raw reply related
* [PATCH v3 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits_le32
From: Corentin Labbe @ 2018-10-24 7:35 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
matthias.bgg, maxime.ripard, michal.lkml, mpe, mporter,
narmstrong, nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb,
wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linux-mediatek, linuxppc-dev, netdev, linux-sunxi,
Corentin Labbe
In-Reply-To: <1540366553-18541-1-git-send-email-clabbe@baylibre.com>
This patch convert meson stmmac glue driver to use all xxxsetbits_le32 functions.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
---
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 ++++++++-----------
1 file changed, 22 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c5979569fd60..abcf65588576 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -23,6 +23,7 @@
#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
#include <linux/stmmac.h>
+#include <linux/setbits.h>
#include "stmmac_platform.h"
@@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
struct clk_gate rgmii_tx_en;
};
-static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
- u32 mask, u32 value)
-{
- u32 data;
-
- data = readl(dwmac->regs + reg);
- data &= ~mask;
- data |= (value & mask);
-
- writel(data, dwmac->regs + reg);
-}
-
static struct clk *meson8b_dwmac_register_clk(struct meson8b_dwmac *dwmac,
const char *name_suffix,
const char **parent_names,
@@ -192,14 +181,13 @@ static int meson8b_set_phy_mode(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* enable RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_RGMII_MODE,
- PRG_ETH0_RGMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE,
+ PRG_ETH0_RGMII_MODE);
break;
case PHY_INTERFACE_MODE_RMII:
/* disable RGMII mode -> enables RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_RGMII_MODE, 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE,
+ 0);
break;
default:
dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -218,15 +206,15 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* enable RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_EXT_PHY_MODE_MASK,
- PRG_ETH0_EXT_RGMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_EXT_PHY_MODE_MASK,
+ PRG_ETH0_EXT_RGMII_MODE);
break;
case PHY_INTERFACE_MODE_RMII:
/* disable RGMII mode -> enables RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_EXT_PHY_MODE_MASK,
- PRG_ETH0_EXT_RMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_EXT_PHY_MODE_MASK,
+ PRG_ETH0_EXT_RMII_MODE);
break;
default:
dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -255,11 +243,11 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* only relevant for RMII mode -> disable in RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_INVERTED_RMII_CLK, 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_INVERTED_RMII_CLK, 0);
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK,
+ tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
/* Configure the 125MHz RGMII TX clock, the IP block changes
* the output automatically (= without us having to configure
@@ -287,13 +275,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RMII:
/* invert internal clk_rmii_i to generate 25/2.5 tx_rx_clk */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_INVERTED_RMII_CLK,
- PRG_ETH0_INVERTED_RMII_CLK);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_INVERTED_RMII_CLK,
+ PRG_ETH0_INVERTED_RMII_CLK);
/* TX clock delay cannot be configured in RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK,
+ 0);
break;
@@ -304,8 +292,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
}
/* enable TX_CLK and PHY_REF_CLK generator */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
- PRG_ETH0_TX_AND_PHY_REF_CLK);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
+ PRG_ETH0_TX_AND_PHY_REF_CLK);
return 0;
}
--
2.18.1
^ permalink raw reply related
* Re: [PATCH RFC v3 0/3] Rlimit for module space
From: Daniel Borkmann @ 2018-10-24 16:04 UTC (permalink / raw)
To: Jessica Yu, Ard Biesheuvel
Cc: Edgecombe, Rick P, linux-kernel@vger.kernel.org,
keescook@chromium.org, linux-arch@vger.kernel.org,
arjan@linux.intel.com, tglx@linutronix.de,
linux-mips@linux-mips.org, linux-s390@vger.kernel.org,
jannh@google.com, x86@kernel.org, kristen@linux.intel.com,
Dock, Deneen T, catalin.marinas@arm.com, mingo@redhat.com,
will.deacon@arm.com,
"kernel-hardening@lists.openwall.com"
In-Reply-To: <20181024150706.jewcclhhh756tupn@linux-8ccs>
[ +Alexei, netdev ]
On 10/24/2018 05:07 PM, Jessica Yu wrote:
> +++ Ard Biesheuvel [23/10/18 08:54 -0300]:
>> On 22 October 2018 at 20:06, Edgecombe, Rick P
>> <rick.p.edgecombe@intel.com> wrote:
[...]
>> I think it is wrong to conflate the two things. Limiting the number of
>> BPF allocations and the limiting number of module allocations are two
>> separate things, and the fact that BPF reuses module_alloc() out of
>> convenience does not mean a single rlimit for both is appropriate.
>
> Hm, I think Ard has a good point. AFAIK, and correct me if I'm wrong,
> users of module_alloc() i.e. kprobes, ftrace, bpf, seem to use it
> because it is an easy way to obtain executable kernel memory (and
> depending on the needs of the architecture, being additionally
> reachable via relative branches) during runtime. The side effect is
> that all these users share the "module" memory space, even though this
> memory region is not exclusively used by modules (well, personally I
> think it technically should be, because seeing module_alloc() usage
> outside of the module loader is kind of a misuse of the module API and
> it's confusing for people who don't know the reason behind its usage
> outside of the module loader).
>
> Right now I'm not sure if it makes sense to impose a blanket limit on
> all module_alloc() allocations when the real motivation behind the
> rlimit is related to BPF, i.e., to stop unprivileged users from
> hogging up all the vmalloc space for modules with JITed BPF filters.
> So the rlimit has more to do with limiting the memory usage of BPF
> filters than it has to do with modules themselves.
>
> I think Ard's suggestion of having a separate bpf_alloc/free API makes
> a lot of sense if we want to keep track of bpf-related allocations
> (and then the rlimit would be enforced for those). Maybe part of the
> module mapping space could be carved out for bpf filters (e.g. have
> BPF_VADDR, BPF_VSIZE, etc like how we have it for modules), or
> continue sharing the region but explicitly define a separate bpf_alloc
> API, depending on an architecture's needs. What do people think?
Hmm, I think here are several issues mixed up at the same time which is just
very confusing, imho:
1) The fact that there are several non-module users of module_alloc()
as Jessica notes such as kprobes, ftrace, bpf, for example. While all of
them are not being modules, they all need to alloc some piece of executable
memory. It's nothing new, this exists for 7 years now since 0a14842f5a3c
("net: filter: Just In Time compiler for x86-64") from BPF side; effectively
that is even /before/ eBPF existed. Having some different API perhaps for all
these users seems to make sense if the goal is not to interfere with modules
themselves. It might also help as a benefit to potentially increase that
memory pool if we're hitting limits at scale which would not be a concern
for normal kernel modules since there's usually just a very few of them
needed (unlike dynamically tracing various kernel parts 24/7 w/ or w/o BPF,
running BPF-seccomp policies, networking BPF policies, etc which need to
scale w/ application or container deployment; so this is of much more
dynamic and unpredictable nature).
2) Then there is rlimit which is proposing to have a "fairer" share among
unprivileged users. I'm not fully sure yet whether rlimit is actually a
nice usable interface for all this. I'd agree that something is needed
on that regard, but I also tend to like Michal Hocko's cgroup proposal,
iow, to have such resource control as part of memory cgroup could be
something to consider _especially_ since it already _exists_ for vmalloc()
backed memory so this should be not much different than that. It sounds
like 2) comes on top of 1).
3) Last but not least, there's a short term fix which is needed independently
of 1) and 2) and should be done immediately which is to account for
unprivileged users and restrict them based on a global configurable
limit such that privileged use keeps functioning, and 2) could enforce
based on the global upper limit, for example. Pending fix is under
https://patchwork.ozlabs.org/patch/987971/ which we intend to ship to
Linus as soon as possible as short term fix. Then something like memcg
can be considered on top since it seems this makes most sense from a
usability point.
Thanks a lot,
Daniel
^ permalink raw reply
* Re: [PATCH net-next 3/4] net: phy-c45: Implement reset/suspend/resume callbacks
From: Jose Abreu @ 2018-10-24 7:50 UTC (permalink / raw)
To: Russell King - ARM Linux, Jose Abreu
Cc: Florian Fainelli, Andrew Lunn, netdev, David S. Miller,
Joao Pinto
In-Reply-To: <20181023105828.GN30658@n2100.armlinux.org.uk>
On 23-10-2018 11:58, Russell King - ARM Linux wrote:
> On Tue, Oct 23, 2018 at 11:28:09AM +0100, Jose Abreu wrote:
>> On 23-10-2018 11:20, Russell King - ARM Linux wrote:
>>> I have no idea what you're proposing there - your patches weren't copied
>>> to me.
>> They just set / unset MDIO_CTRL1_LPOWER bit in PCS. I find that
>> without this remote end doesn't detect link is down ...
>>
>> If it's okay for Generic 10G driver I can submit only this and
>> manually reset PHY in stmmac driver so that I don't need to
>> implement custom PHY driver ...
>
>
>> BTW, I just found out currently Generic 10G Driver is broken
>> without patch 4/4 of this series [1]
>>
>> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_987570_&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=WHDsc6kcWAl4i96Vm5hJ_19IJiuxx_p_Rzo2g-uHDKw&m=zYEDSMZPTCHiPc_3B8buyu0kXnlIEYawnHWAxPrsoSU&s=MlF6I2cBSYkGxgEwNV-hXpXJIvXv_gRYXP-CazjkUSw&e=
> How is it broken - what are the symptoms?
>
> The generic 10G driver is bound not via the normal bus matching and
> phy_bus_match(), but via a manual bind in phy_attach_direct(). This
> calls the probe function, which is phy_probe(), which initialises
> the supported/advertising to the driver's features (which as you note
> are zero.)
Since 719655a14971 ("net: phy: Replace phy driver features u32
with link_mode bitmap"), phy_probe() calls
ethtool_convert_link_mode_to_legacy_u32() with phydrv->features
as argument. Since features are NULL, we will get NULL pointer
dereference.
I guess Generic 10G driver was forgotten in the conversion.
Thanks and Best Regards,
Jose Miguel Abreu
>
> However, phy_attach_direct() goes on to call phy_init_hw(), which
> calls the config_init() method. The config_init() method initialises
> the supported/advertising masks to 10GbaseT. This is (partly) what
> I refer to when I say that the generic 10G support is crippled - it
> only supports this single speed and media.
>
> So the supported/advertising masks should be forced to only 10GbaseT
> at the completion of phy_attach_direct().
>
> The "generic 10G" support doesn't do autonegotiation, configuration
> or link mode forcing. It only assumes 10GbaseT is supported, and
> only checks for the "link up" bits.
>
> It isn't like the non-10G generic PHY support due to history - it
> was added in 2014 by Andy Fleming (see 124059fd53af).
>
> BTW, your patch 1 is wrong as well (introducing phy_update_link()).
> You don't take account that a 10G phy may have alternative ways of
> reading the link (like 88x3310 does, because it has multiple
> instances of AN/PCS/PHYXS at 1k offsets.) All the gen10g_*
> functions are legacy functions for the crippled "generic" 10G
> support.
>
^ permalink raw reply
* Re: [GIT] Networking
From: Kalle Valo @ 2018-10-24 7:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Bjorn Andersson, Govind Singh, David Miller, Andrew Morton,
netdev, Linux Kernel Mailing List, linux-wireless, ath10k,
Jeff Kirsher, Niklas Cassel, Andy Gross, David Brown
In-Reply-To: <CAHk-=wh2H9jE2RXibQgWs3_Q539roj62_gzPXKjnhhKqSFcw2g@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Oct 24, 2018 at 7:01 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> Hmm. Tentatively pulled, but there's something wrong with the Kconfig rules.
>
> Confirmed.
BTW, our emails crossed and more info in the other email[1].
> I did a978a5b8d83f ("net/kconfig: Make QCOM_QMI_HELPERS available when
> COMPILE_TEST") to fix the breakage.
Thanks, though I don't see it yet as I guess you haven't pushed it yet.
Do note that it _might_ conflict the other commit which I suspect is in
also coming to you:
ccfb464cd106 ("soc: qcom: Allow COMPILE_TEST of qcom SoC Kconfigs")
> Why wasn't this noticed in testing?
Mostly bad timing due to my vacation. I did do allmodconfig build but
not sure why I missed the warning, also the kbuild bot didn't report
anything. Jeff did report[2] it last week but I was on vacation at the
time and just came back yesterday and didn't have time to react to it
yet. Really sorry for the mess.
[1] https://lkml.kernel.org/r/87pnvzu7i6.fsf@codeaurora.org
[2] http://lists.infradead.org/pipermail/ath10k/2018-October/012330.html
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Joe Perches @ 2018-10-24 16:23 UTC (permalink / raw)
To: Willy Tarreau, Wang Hai
Cc: edumazet, davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20181024155739.GA25314@1wt.eu>
On Wed, 2018-10-24 at 17:57 +0200, Willy Tarreau wrote:
> On Wed, Oct 24, 2018 at 11:47:29PM +0800, Wang Hai wrote:
> > To determine whether len is less than zero, it should be put before
> > the function min_t, because the return value of min_t is not likely
> > to be less than zero.
>
> Huh? First, the <0 test is made on "len", not "min_t", so it still
> is signed. Second, you're in fact completely removing the test here,
> look :
>
> > struct net *net = sock_net(sk);
> > int val, len;
> >
> > + len = min_t(unsigned int, len, sizeof(int));
> > +
>
> len is used uninitialized here, so the result is undefined.
>
> > if (get_user(len, optlen))
> > return -EFAULT;
>
> Then it gets overridden by get_user()
>
> > - len = min_t(unsigned int, len, sizeof(int));
> > -
>
> Then its positive values are not bounded anymore since you moved the test.
Not quite.
Problem here is negative values are tested as
large positive values and limited to 4
ie:
ien len = -1,
len = min_t(unsigned int, len, sizeof(int));
len is now 4
> > if (len < 0)
> > return -EINVAL;
So this test len < 0 could be moved up above min_t
> Then only negative values are dropped. So unless I'm missing something
> obvious, you're just allowing len to be as large as 2GB-1 based on the
> user's fed optlen.
>
> Am I wrong ?
>
> Willychee
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Willy Tarreau @ 2018-10-24 16:32 UTC (permalink / raw)
To: Joe Perches
Cc: Wang Hai, edumazet, davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <60f08664db5751949ddfb34666bfda77f99682f1.camel@perches.com>
On Wed, Oct 24, 2018 at 09:23:19AM -0700, Joe Perches wrote:
> On Wed, 2018-10-24 at 17:57 +0200, Willy Tarreau wrote:
> > On Wed, Oct 24, 2018 at 11:47:29PM +0800, Wang Hai wrote:
> > > To determine whether len is less than zero, it should be put before
> > > the function min_t, because the return value of min_t is not likely
> > > to be less than zero.
> >
> > Huh? First, the <0 test is made on "len", not "min_t", so it still
> > is signed. Second, you're in fact completely removing the test here,
> > look :
> >
> > > struct net *net = sock_net(sk);
> > > int val, len;
> > >
> > > + len = min_t(unsigned int, len, sizeof(int));
> > > +
> >
> > len is used uninitialized here, so the result is undefined.
> >
> > > if (get_user(len, optlen))
> > > return -EFAULT;
> >
> > Then it gets overridden by get_user()
> >
> > > - len = min_t(unsigned int, len, sizeof(int));
> > > -
> >
> > Then its positive values are not bounded anymore since you moved the test.
>
> Not quite.
>
> Problem here is negative values are tested as
> large positive values and limited to 4
>
> ie:
> ien len = -1,
> len = min_t(unsigned int, len, sizeof(int));
>
> len is now 4
>
> > > if (len < 0)
> > > return -EINVAL;
>
> So this test len < 0 could be moved up above min_t
It could indeed, or we could also have min_t() done on an int instead
of an unsigned int and this would avoid the need to shuffle the code
around and open a huge hole like this one.
Willy
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Joe Perches @ 2018-10-24 16:54 UTC (permalink / raw)
To: Willy Tarreau
Cc: Wang Hai, edumazet, davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20181024163230.GA25382@1wt.eu>
On Wed, 2018-10-24 at 18:32 +0200, Willy Tarreau wrote:
> On Wed, Oct 24, 2018 at 09:23:19AM -0700, Joe Perches wrote:
> > On Wed, 2018-10-24 at 17:57 +0200, Willy Tarreau wrote:
> > > On Wed, Oct 24, 2018 at 11:47:29PM +0800, Wang Hai wrote:
> > > > To determine whether len is less than zero, it should be put before
> > > > the function min_t, because the return value of min_t is not likely
> > > > to be less than zero.
> > >
> > > Huh? First, the <0 test is made on "len", not "min_t", so it still
> > > is signed. Second, you're in fact completely removing the test here,
> > > look :
> > >
> > > > struct net *net = sock_net(sk);
> > > > int val, len;
> > > >
> > > > + len = min_t(unsigned int, len, sizeof(int));
> > > > +
> > >
> > > len is used uninitialized here, so the result is undefined.
> > >
> > > > if (get_user(len, optlen))
> > > > return -EFAULT;
> > >
> > > Then it gets overridden by get_user()
> > >
> > > > - len = min_t(unsigned int, len, sizeof(int));
> > > > -
> > >
> > > Then its positive values are not bounded anymore since you moved the test.
> >
> > Not quite.
> >
> > Problem here is negative values are tested as
> > large positive values and limited to 4
> >
> > ie:
> > ien len = -1,
> > len = min_t(unsigned int, len, sizeof(int));
> >
> > len is now 4
> >
> > > > if (len < 0)
> > > > return -EINVAL;
> >
> > So this test len < 0 could be moved up above min_t
>
> It could indeed, or we could also have min_t() done on an int instead
> of an unsigned int and this would avoid the need to shuffle the code
> around and open a huge hole like this one.
I think if the point is to test for negative numbers,
it's clearer to do that before using min_t.and it's
probably clearer not to use min_t at all.
if (get_user(len, optlen))
return -EFAULT;
if (len < 0)
return -EINVAL;
if (len > sizeof(int))
len = sizeof(int);
^ permalink raw reply
* [PATCH net-next] octeontx2-af: Copy the right amount of memory
From: Dan Carpenter @ 2018-10-24 8:32 UTC (permalink / raw)
To: Sunil Goutham
Cc: Linu Cherian, Geetha sowjanya, Jerin Jacob, David S. Miller,
netdev, kernel-janitors
This is a copy and paste bug where we copied the sizeof() from the chunk
before. We're copying more data than intended but the destination is a
union so it doesn't cause memory corruption.
Fixes: ffb0abd7e9cb ("octeontx2-af: NIX AQ instruction enqueue support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 8890c95831ca..a4eac3b9ee72 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -573,7 +573,7 @@ static int rvu_nix_aq_enq_inst(struct rvu *rvu, struct nix_aq_enq_req *req,
sizeof(struct nix_cq_ctx_s));
else if (req->ctype == NIX_AQ_CTYPE_RSS)
memcpy(&rsp->rss, ctx,
- sizeof(struct nix_cq_ctx_s));
+ sizeof(struct nix_rsse_s));
else if (req->ctype == NIX_AQ_CTYPE_MCE)
memcpy(&rsp->mce, ctx,
sizeof(struct nix_rx_mce_s));
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] Change judgment len position
From: Eric Dumazet @ 2018-10-24 17:03 UTC (permalink / raw)
To: joe
Cc: Willy Tarreau, wanghaifine, David Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev, LKML
In-Reply-To: <bbc4eba31b89cb487962c0a5980a12c53b1aa58b.camel@perches.com>
On Wed, Oct 24, 2018 at 9:54 AM Joe Perches <joe@perches.com> wrote:
> I think if the point is to test for negative numbers,
> it's clearer to do that before using min_t.and it's
> probably clearer not to use min_t at all.
>
...
>
> if (len > sizeof(int))
> len = sizeof(int);
It is a matter of taste really, I know some people (like me) sometimes
mixes min() and max()
I would suggest that if someones wants to change the current code, a
corresponding test
would be added in tools/testing/selftests/net
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: David Miller @ 2018-10-24 17:10 UTC (permalink / raw)
To: wanghaifine; +Cc: edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20181024154729.5312-1-wanghaifine@gmail.com>
From: Wang Hai <wanghaifine@gmail.com>
Date: Wed, 24 Oct 2018 23:47:29 +0800
> To determine whether len is less than zero, it should be put before
> the function min_t, because the return value of min_t is not likely
> to be less than zero.
>
> Signed-off-by: Wang Hai <wanghaifine@gmail.com>
> ---
> net/ipv4/tcp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 10c624639..49af9fdc3 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3301,11 +3301,11 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> struct net *net = sock_net(sk);
> int val, len;
>
> + len = min_t(unsigned int, len, sizeof(int));
> +
> if (get_user(len, optlen))
> return -EFAULT;
You can't be serious?
'len' has no value before the get_user() call.
^ permalink raw reply
* Re: [net-next 01/11] igc: Add skeletal frame for Intel(R) 2.5G Ethernet Controller support
From: Neftin, Sasha @ 2018-10-24 8:50 UTC (permalink / raw)
To: Jakub Kicinski, Jeff Kirsher
Cc: davem, netdev, nhorman, sassmann, sasha.neftin
In-Reply-To: <20181018101405.4998dc01@cakuba.netronome.com>
On 10/18/2018 20:14, Jakub Kicinski wrote:
> On Wed, 17 Oct 2018 15:23:12 -0700, Jeff Kirsher wrote:
>> From: Sasha Neftin <sasha.neftin@intel.com>
>>
>> This patch adds the beginning framework onto which I am going to add
>> the igc driver which supports the Intel(R) I225-LM/I225-V 2.5G
>> Ethernet Controller.
>>
>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> bunch of minor nit picks
>
>> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
>> new file mode 100644
>> index 000000000000..afe595cfcf63
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/igc/igc.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (c) 2018 Intel Corporation */
>> +
>> +#ifndef _IGC_H_
>> +#define _IGC_H_
>> +
>> +#include <linux/kobject.h>
>> +
>> +#include <linux/pci.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/vmalloc.h>
>> +
>> +#include <linux/ethtool.h>
>> +
>> +#include <linux/sctp.h>
>> +
>> +#define IGC_ERR(args...) pr_err("igc: " args)
>
> Looks like you're reimplementing pr_fmt()
>
yes, it is convenient for a debug. Same methodological approach I saw in
other drivers.
>> +#define PFX "igc: "
>> +
>> +#include <linux/timecounter.h>
>> +#include <linux/net_tstamp.h>
>> +#include <linux/ptp_clock_kernel.h>
>
> Splitting the includes looks fairly weird. Also, you're not using any
> of them here.
>
Good catch. I'll remove splits and unused "include"'s. All "include"'s
will be add per demand. I will send patch to fix that.
>> +/* main */
>> +extern char igc_driver_name[];
>> +extern char igc_driver_version[];
>> +
>> +#endif /* _IGC_H_ */
>> diff --git a/drivers/net/ethernet/intel/igc/igc_hw.h b/drivers/net/ethernet/intel/igc/igc_hw.h
>> new file mode 100644
>> index 000000000000..aa68b4516700
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/igc/igc_hw.h
>> @@ -0,0 +1,10 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (c) 2018 Intel Corporation */
>> +
>> +#ifndef _IGC_HW_H_
>> +#define _IGC_HW_H_
>> +
>> +#define IGC_DEV_ID_I225_LM 0x15F2
>> +#define IGC_DEV_ID_I225_V 0x15F3
>> +
>> +#endif /* _IGC_HW_H_ */
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
>> new file mode 100644
>> index 000000000000..753749ce5ae0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -0,0 +1,146 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2018 Intel Corporation */
>> +
>> +#include <linux/module.h>
>> +#include <linux/types.h>
>> +
>> +#include "igc.h"
>> +#include "igc_hw.h"
>> +
>> +#define DRV_VERSION "0.0.1-k"
>
> You can just use the kernel version, it works pretty well in presence
> of backports.
>
Good idea, I think I will do it too.
>> +#define DRV_SUMMARY "Intel(R) 2.5G Ethernet Linux Driver"
>> +
>> +MODULE_AUTHOR("Intel Corporation, <linux.nics@intel.com>");
>> +MODULE_DESCRIPTION(DRV_SUMMARY);
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_VERSION(DRV_VERSION);
>> +
>> +char igc_driver_name[] = "igc";
>> +char igc_driver_version[] = DRV_VERSION;
>> +static const char igc_driver_string[] = DRV_SUMMARY;
>> +static const char igc_copyright[] =
>> + "Copyright(c) 2018 Intel Corporation.";
>> +
>> +static const struct pci_device_id igc_pci_tbl[] = {
>> + { PCI_VDEVICE(INTEL, IGC_DEV_ID_I225_LM) },
>> + { PCI_VDEVICE(INTEL, IGC_DEV_ID_I225_V) },
>> + /* required last entry */
>> + {0, }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(pci, igc_pci_tbl);
>> +
>> +/**
>> + * igc_probe - Device Initialization Routine
>> + * @pdev: PCI device information struct
>> + * @ent: entry in igc_pci_tbl
>> + *
>> + * Returns 0 on success, negative on failure
>> + *
>> + * igc_probe initializes an adapter identified by a pci_dev structure.
>> + * The OS initialization, configuring the adapter private structure,
>> + * and a hardware reset occur.
>> + */
>> +static int igc_probe(struct pci_dev *pdev,
>> + const struct pci_device_id *ent)
>> +{
>> + int err, pci_using_dac;
>> +
>> + err = pci_enable_device_mem(pdev);
>> + if (err)
>> + return err;
>> +
>> + pci_using_dac = 0;
>> + err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
>> + if (!err) {
>> + err = dma_set_coherent_mask(&pdev->dev,
>> + DMA_BIT_MASK(64));
>> + if (!err)
>> + pci_using_dac = 1;
>
> You never use this pci_using_dac. dma_set_mask_and_coherent() maybe?
>
Right. I saw already somebody sent out patch to fix that.
>> + } else {
>> + err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
>> + if (err) {
>> + err = dma_set_coherent_mask(&pdev->dev,
>> + DMA_BIT_MASK(32));
>> + if (err) {
>> + IGC_ERR("Wrong DMA configuration, aborting\n");
>> + goto err_dma;
>> + }
>> + }
>> + }
>> +
>> + err = pci_request_selected_regions(pdev,
>> + pci_select_bars(pdev,
>> + IORESOURCE_MEM),
>> + igc_driver_name);
>> + if (err)
>> + goto err_pci_reg;
>> +
>> + pci_set_master(pdev);
>> + err = pci_save_state(pdev);
>
> And you never look at err?
>
Same as above.
>> + return 0;
>> +
>> +err_pci_reg:
>> +err_dma:
>
> The error label should be named after what it points to, not where it's
> coming from.
>
>> + pci_disable_device(pdev);
>> + return err;
>> +}
>> +
>> +/**
>> + * igc_remove - Device Removal Routine
>> + * @pdev: PCI device information struct
>> + *
>> + * igc_remove is called by the PCI subsystem to alert the driver
>> + * that it should release a PCI device. This could be caused by a
>> + * Hot-Plug event, or because the driver is going to be removed from
>> + * memory.
>> + */
>> +static void igc_remove(struct pci_dev *pdev)
>> +{
>> + pci_release_selected_regions(pdev,
>> + pci_select_bars(pdev, IORESOURCE_MEM));
>> +
>> + pci_disable_device(pdev);
>> +}
>> +
>> +static struct pci_driver igc_driver = {
>> + .name = igc_driver_name,
>> + .id_table = igc_pci_tbl,
>> + .probe = igc_probe,
>> + .remove = igc_remove,
>> +};
>> +
>> +/**
>> + * igc_init_module - Driver Registration Routine
>> + *
>> + * igc_init_module is the first routine called when the driver is
>> + * loaded. All it does is register with the PCI subsystem.
>> + */
>> +static int __init igc_init_module(void)
>> +{
>> + int ret;
>> +
>> + pr_info("%s - version %s\n",
>> + igc_driver_string, igc_driver_version);
>> +
>> + pr_info("%s\n", igc_copyright);
>> +
>> + ret = pci_register_driver(&igc_driver);
>> + return ret;
>
> Why the variable?
>
we can return 'pci_register_driver' here, but variable is more clearly.
>> +}
>> +
>> +module_init(igc_init_module);
>> +
>> +/**
>> + * igc_exit_module - Driver Exit Cleanup Routine
>> + *
>> + * igc_exit_module is called just before the driver is removed
>> + * from memory.
>> + */
>> +static void __exit igc_exit_module(void)
>> +{
>> + pci_unregister_driver(&igc_driver);
>> +}
>> +
>> +module_exit(igc_exit_module);
>> +/* igc_main.c */
>
> I'd argue most editors make it fairly clear which file one is
> editing, hence making this sort of comments entirely superfluous :)
>
Thanks for your comments.
^ permalink raw reply
* Re: [PATCH] Change judgment len position
From: Joe Perches @ 2018-10-24 17:18 UTC (permalink / raw)
To: Eric Dumazet
Cc: Willy Tarreau, wanghaifine, David Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, netdev, LKML
In-Reply-To: <CANn89iLH=VP1i=KS5QV1x41EpQM5-o1TJfDh01Y++bMpFpfBRg@mail.gmail.com>
On Wed, 2018-10-24 at 10:03 -0700, Eric Dumazet wrote:
> On Wed, Oct 24, 2018 at 9:54 AM Joe Perches <joe@perches.com> wrote:
>
> > I think if the point is to test for negative numbers,
> > it's clearer to do that before using min_t.and it's
> > probably clearer not to use min_t at all.
> >
>
> ...
>
> > if (len > sizeof(int))
> > len = sizeof(int);
>
> It is a matter of taste really,
Agree and hence my use of 'I think' above.
> I know some people (like me) sometimes
> mixes min() and max()
Not quite sure what you mean here by mixes.
mix up? If so, the < > inversions probably
have about the same error rate.
And I suppose there are cases where the
always set of len in uses like
len = min(len, 4);
are more costly (len being in a slow write
speed area of memory or some such) than the
other style of
if (len < 4)
len = 4;
I think that min() is easier to read in most
cases.
> I would suggest that if someones wants to change the current code, a
> corresponding test would be added in tools/testing/selftests/net?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox