Linux CAN drivers development
 help / color / mirror / Atom feed
* Re: [PATCH]: canfdtest typo while waiting for tv_stop time
From: Marc Kleine-Budde @ 2016-11-21 10:36 UTC (permalink / raw)
  To: Stephane Grosjean, linux-can@vger.kernel.org
In-Reply-To: <5bd8e04f-606c-0b25-dab9-7a1090ca8810@peak-system.com>


[-- Attachment #1.1: Type: text/plain, Size: 1304 bytes --]

On 11/16/2016 11:46 AM, Stephane Grosjean wrote:
> After a few complaint about 100% CPU usage of canfdtest over our PCI 
> -like boards, I have had a look to the source file, and think I have 
> found the issue in:
> 
> diff --git a/canfdtest.c b/canfdtest.c
> index 2407027..d7ba740 100644
> --- a/canfdtest.c
> +++ b/canfdtest.c
> @@ -239,7 +239,7 @@ static int can_echo_dut(void)
>                                  }
>                                  gettimeofday(&tvn, NULL);
>                                  while ((tv_stop.tv_sec > tvn.tv_sec) ||
> -                                      ((tv_stop.tv_sec = tvn.tv_sec) &&
> +                                      ((tv_stop.tv_sec == tvn.tv_sec) &&
>                                          (tv_stop.tv_usec >= tvn.tv_usec)))
>                                          gettimeofday(&tvn, NULL);
>                          }
> 
> Am I right?

Yes. But it still does a busywait. I've reviewed Ramesh's changes, that
will fix the busywait.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH]: canfdtest typo while waiting for tv_stop time
From: Marc Kleine-Budde @ 2016-11-21 10:37 UTC (permalink / raw)
  To: Ramesh Shanmugasundaram, Stephane Grosjean,
	linux-can@vger.kernel.org
In-Reply-To: <SG2PR06MB10387876750D66073C215FF4C3B10@SG2PR06MB1038.apcprd06.prod.outlook.com>


[-- Attachment #1.1: Type: text/plain, Size: 1516 bytes --]

On 11/17/2016 09:09 AM, Ramesh Shanmugasundaram wrote:
>> Subject: [PATCH]: canfdtest typo while waiting for tv_stop time
>>
>> Hello Mark,
>>
>> After a few complaint about 100% CPU usage of canfdtest over our PCI -like
>> boards, I have had a look to the source file, and think I have found the
>> issue in:
>>
>> diff --git a/canfdtest.c b/canfdtest.c
>> index 2407027..d7ba740 100644
>> --- a/canfdtest.c
>> +++ b/canfdtest.c
>> @@ -239,7 +239,7 @@ static int can_echo_dut(void)
>>                                  }
>>                                  gettimeofday(&tvn, NULL);
>>                                  while ((tv_stop.tv_sec > tvn.tv_sec) ||
>> -                                      ((tv_stop.tv_sec = tvn.tv_sec) &&
>> +                                      ((tv_stop.tv_sec == tvn.tv_sec)
>> + &&
>>                                          (tv_stop.tv_usec >=
>> tvn.tv_usec)))
>>                                          gettimeofday(&tvn, NULL);
>>                          }
>>
>> Am I right?
> 
> There is one more bug on that piece of code. I submitted a bug & a pull request fixing it
> 
> https://github.com/linux-can/can-utils/pull/15

See github for the review.

Thanks,
Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* net/can: use-after-free in bcm_rx_thr_flush
From: Andrey Konovalov @ 2016-11-22  9:22 UTC (permalink / raw)
  To: Oliver Hartkopp, Marc Kleine-Budde, David S. Miller, linux-can,
	netdev, LKML
  Cc: Dmitry Vyukov, Alexander Potapenko, Kostya Serebryany,
	Eric Dumazet, syzkaller

[-- Attachment #1: Type: text/plain, Size: 5259 bytes --]

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

A reproducer is attached.
You may need to run it a few times.

On commit 9c763584b7c8911106bb77af7e648bef09af9d80 (4.9-rc6, Nov 20).

==================================================================
BUG: KASAN: use-after-free in bcm_rx_thr_flush+0x284/0x2b0
Read of size 1 at addr ffff88006c1faae5 by task a.out/3874

page:ffffea0001b07e80 count:1 mapcount:0 mapping:          (null) index:0x0
flags: 0x100000000000080(slab)
page dumped because: kasan: bad access detected

CPU: 1 PID: 3874 Comm: a.out Not tainted 4.9.0-rc6+ #427
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff88006ab07900 ffffffff81b472e4 ffff88006ab07990 ffff88006c1faae5
 00000000000000fa 00000000000000fb ffff88006ab07980 ffffffff8150ad42
 ffff88006323ce58 0000000000000246 ffff880068ca8000 0000000000000282
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81b472e4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
 [<     inline     >] describe_address mm/kasan/report.c:259
 [<ffffffff8150ad42>] kasan_report_error+0x122/0x560 mm/kasan/report.c:365
 [<     inline     >] kasan_report mm/kasan/report.c:387
 [<ffffffff8150b1be>] __asan_report_load1_noabort+0x3e/0x40
mm/kasan/report.c:405
 [<     inline     >] bcm_rx_do_flush net/can/bcm.c:589
 [<ffffffff83577e04>] bcm_rx_thr_flush+0x284/0x2b0 net/can/bcm.c:612
 [<     inline     >] bcm_rx_setup net/can/bcm.c:1199
 [<ffffffff83578b36>] bcm_sendmsg+0xbb6/0x30e0 net/can/bcm.c:1351
 [<     inline     >] sock_sendmsg_nosec net/socket.c:621
 [<ffffffff82b7176c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
 [<ffffffff82b73651>] ___sys_sendmsg+0x771/0x8b0 net/socket.c:1954
 [<ffffffff82b7563e>] __sys_sendmsg+0xce/0x170 net/socket.c:1988
 [<     inline     >] SYSC_sendmsg net/socket.c:1999
 [<ffffffff82b7570d>] SyS_sendmsg+0x2d/0x50 net/socket.c:1995
 [<ffffffff83fc4301>] entry_SYSCALL_64_fastpath+0x1f/0xc2
arch/x86/entry/entry_64.S:209

The buggy address belongs to the object at ffff88006c1faae0
 which belongs to the cache kmalloc-32 of size 32
The buggy address ffff88006c1faae5 is located 5 bytes inside
 of 32-byte region [ffff88006c1faae0, ffff88006c1fab00)

Freed by task 2013:
 [<ffffffff8107e236>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 [<ffffffff81509e56>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [<     inline     >] set_track mm/kasan/kasan.c:507
 [<ffffffff8150a6b3>] kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:571
 [<     inline     >] slab_free_hook mm/slub.c:1352
 [<     inline     >] slab_free_freelist_hook mm/slub.c:1374
 [<     inline     >] slab_free mm/slub.c:2951
 [<ffffffff81506b98>] kfree+0xe8/0x2b0 mm/slub.c:3871
 [<ffffffff819dd8c1>] selinux_cred_free+0x51/0x80 security/selinux/hooks.c:3725
 [<ffffffff819ce358>] security_cred_free+0x48/0x80 security/security.c:907
 [<ffffffff8117e27d>] put_cred_rcu+0xed/0x390 kernel/cred.c:116
 [<     inline     >] __rcu_reclaim kernel/rcu/rcu.h:118
 [<     inline     >] rcu_do_batch kernel/rcu/tree.c:2776
 [<     inline     >] invoke_rcu_callbacks kernel/rcu/tree.c:3040
 [<     inline     >] __rcu_process_callbacks kernel/rcu/tree.c:3007
 [<ffffffff8125dfe0>] rcu_process_callbacks+0xa40/0x1190 kernel/rcu/tree.c:3024
 [<ffffffff83fc70af>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284

Allocated by task 1826:
 [<ffffffff8107e236>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 [<ffffffff81509e56>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [<     inline     >] set_track mm/kasan/kasan.c:507
 [<ffffffff8150a0cb>] kasan_kmalloc+0xab/0xe0 mm/kasan/kasan.c:598
 [<ffffffff8150a632>] kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:537
 [<     inline     >] slab_post_alloc_hook mm/slab.h:417
 [<     inline     >] slab_alloc_node mm/slub.c:2708
 [<     inline     >] slab_alloc mm/slub.c:2716
 [<ffffffff815090ef>] __kmalloc_track_caller+0xcf/0x2a0 mm/slub.c:4240
 [<ffffffff8146bf84>] kmemdup+0x24/0x50 mm/util.c:113
 [<ffffffff819dcbe9>] selinux_cred_prepare+0x49/0xb0
security/selinux/hooks.c:3739
 [<ffffffff819ce40d>] security_prepare_creds+0x7d/0xb0 security/security.c:912
 [<ffffffff8117fab3>] prepare_creds+0x243/0x340 kernel/cred.c:277
 [<ffffffff81181bab>] copy_creds+0x7b/0x5c0 kernel/cred.c:343
 [<ffffffff81109c6e>] copy_process.part.45+0x86e/0x5b50 kernel/fork.c:1529
 [<     inline     >] copy_process kernel/fork.c:1479
 [<ffffffff8110f2fa>] _do_fork+0x1ba/0xcc0 kernel/fork.c:1933
 [<     inline     >] SYSC_clone kernel/fork.c:2043
 [<ffffffff8110fed7>] SyS_clone+0x37/0x50 kernel/fork.c:2037
 [<ffffffff81006465>] do_syscall_64+0x195/0x490 arch/x86/entry/common.c:280
 [<ffffffff83fc43c9>] return_from_SYSCALL_64+0x0/0x7a
arch/x86/entry/entry_64.S:251

Memory state around the buggy address:
 ffff88006c1fa980: fc fc fb fb fb fb fc fc fb fb fb fb fc fc fb fb
 ffff88006c1faa00: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
>ffff88006c1faa80: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
                                                       ^
 ffff88006c1fab00: fc fc fb fb fb fb fc fc 00 00 00 00 fc fc 00 00
 ffff88006c1fab80: 00 00 fc fc fb fb fb fb fc fc fb fb fb fb fc fc
==================================================================

Thanks!

[-- Attachment #2: bcm-rx-uaf-poc.c --]
[-- Type: text/x-csrc, Size: 7860 bytes --]

// autogenerated by syzkaller (http://github.com/google/syzkaller)

#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_syz_fuse_mount
#define __NR_syz_fuse_mount 1000004
#endif
#ifndef __NR_syz_fuseblk_mount
#define __NR_syz_fuseblk_mount 1000005
#endif
#ifndef __NR_syz_open_pts
#define __NR_syz_open_pts 1000003
#endif
#ifndef __NR_syz_test
#define __NR_syz_test 1000001
#endif
#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_connect
#define __NR_connect 42
#endif
#ifndef __NR_sendmsg
#define __NR_sendmsg 46
#endif
#ifndef __NR_syz_open_dev
#define __NR_syz_open_dev 1000002
#endif

#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <net/if_arp.h>

#include <errno.h>
#include <error.h>
#include <fcntl.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

__thread int skip_segv;
__thread jmp_buf segv_env;

static void segv_handler(int sig, siginfo_t* info, void* uctx)
{
  if (__atomic_load_n(&skip_segv, __ATOMIC_RELAXED))
    _longjmp(segv_env, 1);
  exit(sig);
}

static void install_segv_handler()
{
  struct sigaction sa;
  memset(&sa, 0, sizeof(sa));
  sa.sa_sigaction = segv_handler;
  sa.sa_flags = SA_NODEFER | SA_SIGINFO;
  sigaction(SIGSEGV, &sa, NULL);
  sigaction(SIGBUS, &sa, NULL);
}

#define NONFAILING(...)                                                \
  {                                                                    \
    __atomic_fetch_add(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
    if (_setjmp(segv_env) == 0) {                                      \
      __VA_ARGS__;                                                     \
    }                                                                  \
    __atomic_fetch_sub(&skip_segv, 1, __ATOMIC_SEQ_CST);               \
  }

static uintptr_t syz_open_dev(uintptr_t a0, uintptr_t a1, uintptr_t a2)
{
  if (a0 == 0xc || a0 == 0xb) {

    char buf[128];
    sprintf(buf, "/dev/%s/%d:%d", a0 == 0xc ? "char" : "block",
            (uint8_t)a1, (uint8_t)a2);
    return open(buf, O_RDWR, 0);
  } else {

    char buf[1024];
    char* hash;
    strncpy(buf, (char*)a0, sizeof(buf));
    buf[sizeof(buf) - 1] = 0;
    while ((hash = strchr(buf, '#'))) {
      *hash = '0' + (char)(a1 % 10);
      a1 /= 10;
    }
    return open(buf, a2, 0);
  }
}

static uintptr_t syz_open_pts(uintptr_t a0, uintptr_t a1)
{

  int ptyno = 0;
  if (ioctl(a0, TIOCGPTN, &ptyno))
    return -1;
  char buf[128];
  sprintf(buf, "/dev/pts/%d", ptyno);
  return open(buf, a1, 0);
}

static uintptr_t syz_fuse_mount(uintptr_t a0, uintptr_t a1,
                                uintptr_t a2, uintptr_t a3,
                                uintptr_t a4, uintptr_t a5)
{

  uint64_t target = a0;
  uint64_t mode = a1;
  uint64_t uid = a2;
  uint64_t gid = a3;
  uint64_t maxread = a4;
  uint64_t flags = a5;

  int fd = open("/dev/fuse", O_RDWR);
  if (fd == -1)
    return fd;
  char buf[1024];
  sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
          (long)uid, (long)gid, (unsigned)mode & ~3u);
  if (maxread != 0)
    sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
  if (mode & 1)
    strcat(buf, ",default_permissions");
  if (mode & 2)
    strcat(buf, ",allow_other");
  syscall(SYS_mount, "", target, "fuse", flags, buf);

  return fd;
}

static uintptr_t syz_fuseblk_mount(uintptr_t a0, uintptr_t a1,
                                   uintptr_t a2, uintptr_t a3,
                                   uintptr_t a4, uintptr_t a5,
                                   uintptr_t a6, uintptr_t a7)
{

  uint64_t target = a0;
  uint64_t blkdev = a1;
  uint64_t mode = a2;
  uint64_t uid = a3;
  uint64_t gid = a4;
  uint64_t maxread = a5;
  uint64_t blksize = a6;
  uint64_t flags = a7;

  int fd = open("/dev/fuse", O_RDWR);
  if (fd == -1)
    return fd;
  if (syscall(SYS_mknodat, AT_FDCWD, blkdev, S_IFBLK, makedev(7, 199)))
    return fd;
  char buf[256];
  sprintf(buf, "fd=%d,user_id=%ld,group_id=%ld,rootmode=0%o", fd,
          (long)uid, (long)gid, (unsigned)mode & ~3u);
  if (maxread != 0)
    sprintf(buf + strlen(buf), ",max_read=%ld", (long)maxread);
  if (blksize != 0)
    sprintf(buf + strlen(buf), ",blksize=%ld", (long)blksize);
  if (mode & 1)
    strcat(buf, ",default_permissions");
  if (mode & 2)
    strcat(buf, ",allow_other");
  syscall(SYS_mount, blkdev, target, "fuseblk", flags, buf);

  return fd;
}

static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
                                 uintptr_t a2, uintptr_t a3,
                                 uintptr_t a4, uintptr_t a5,
                                 uintptr_t a6, uintptr_t a7,
                                 uintptr_t a8)
{
  switch (nr) {
  default:
    return syscall(nr, a0, a1, a2, a3, a4, a5);
  case __NR_syz_test:
    return 0;
  case __NR_syz_open_dev:
    return syz_open_dev(a0, a1, a2);
  case __NR_syz_open_pts:
    return syz_open_pts(a0, a1);
  case __NR_syz_fuse_mount:
    return syz_fuse_mount(a0, a1, a2, a3, a4, a5);
  case __NR_syz_fuseblk_mount:
    return syz_fuseblk_mount(a0, a1, a2, a3, a4, a5, a6, a7);
  }
}

long r[25];

int main()
{
  install_segv_handler();
  memset(r, -1, sizeof(r));
  r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0xf60000ul, 0x3ul,
                         0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
  r[1] = execute_syscall(__NR_socket, 0x1dul, 0x80002ul, 0x2ul, 0, 0, 0,
                         0, 0, 0);
  NONFAILING(*(uint16_t*)0x20f57000 = (uint16_t)0x27);
  NONFAILING(*(uint32_t*)0x20f57004 = (uint32_t)0x0);
  NONFAILING(*(uint32_t*)0x20f57008 = (uint32_t)0x0);
  NONFAILING(*(uint32_t*)0x20f5700c = (uint32_t)0x0);
  NONFAILING(*(uint8_t*)0x20f57010 = (uint8_t)0x0);
  NONFAILING(*(uint8_t*)0x20f57011 = (uint8_t)0x0);
  NONFAILING(memcpy(
      (void*)0x20f57012,
      "\x34\x1b\x3a\x01\xb2\x57\x84\x9c\xa1\xd7\xd1\xff\x9f\x99\x9d\x81"
      "\x27\xb1\x85\xf8\x8d\x1d\x77\x5d\x59\xc8\x8a\x3a\xa6\xa8\xdd\xac"
      "\xdf\x2b\xdc\x32\x4e\xa6\x57\x8a\x21\xb8\x51\x14\x61\x01\x86\xc3"
      "\x81\x7c\x34\xb0\x5e\xaf\xfd\x2c\x3f\x54\xf5\x7f\xa8\x1b\xa0",
      63));
  NONFAILING(*(uint64_t*)0x20f57058 = (uint64_t)0x0);
  r[10] = execute_syscall(__NR_connect, r[1], 0x20f57000ul, 0x60ul, 0,
                          0, 0, 0, 0, 0);
  NONFAILING(*(uint64_t*)0x20b05000 = (uint64_t)0x20f55000);
  NONFAILING(*(uint32_t*)0x20b05008 = (uint32_t)0x0);
  NONFAILING(*(uint64_t*)0x20b05010 = (uint64_t)0x20008fe0);
  NONFAILING(*(uint64_t*)0x20b05018 = (uint64_t)0x2);
  NONFAILING(*(uint64_t*)0x20b05020 = (uint64_t)0x20f54000);
  NONFAILING(*(uint64_t*)0x20b05028 = (uint64_t)0x0);
  NONFAILING(*(uint32_t*)0x20b05030 = (uint32_t)0x0);
  NONFAILING(*(uint64_t*)0x20008fe0 = (uint64_t)0x20d5fff1);
  NONFAILING(*(uint64_t*)0x20008fe8 = (uint64_t)0xf);
  NONFAILING(*(uint64_t*)0x20008ff0 = (uint64_t)0x20f55000);
  NONFAILING(*(uint64_t*)0x20008ff8 = (uint64_t)0x69);
  NONFAILING(memcpy(
      (void*)0x20d5fff1,
      "\x05\x00\x00\x00\x8d\x13\x00\x00\x00\x00\x17\x14\xb7\x7e\xa6",
      15));
  NONFAILING(memcpy(
      (void*)0x20f55000,
      "\x12\x6f\x39\xb6\x5b\x4e\xed\x90\x77\xe0\x54\xbf\xb6\xb2\x41\xd7"
      "\x36\x5d\x58\xfa\xa8\x32\x7a\x6d\x25\x89\x01\x00\xdd\x00\xc5\x89"
      "\x07\xec\xc2\x76\x8d\x02\x00\x00\x00\x10\xb4\x27\xab\x6c\x2a\x41"
      "\xe2\x54\x47\xcc\x08\xca\x75\x2a\x03\x89\xd3\x04\x71\x3f\x75\x90"
      "\xf4\xda\xc6\xd9\xa7\x50\xff\xe8\x3e\xff\xcd\x31\x1b\xa2\x0a\xee"
      "\x8a\x72\x6b\xda\x74\x75\x92\xbf\xad\xf0\x71\xb9\xb7\x70\x04\xbb"
      "\x58\x40\x7d\x50\x14\x6b\xd7\xc2\x60",
      105));
  r[24] = execute_syscall(__NR_sendmsg, r[1], 0x20b05000ul, 0x0ul, 0, 0,
                          0, 0, 0, 0);
  return 0;
}

^ permalink raw reply

* Re: net/can: use-after-free in bcm_rx_thr_flush
From: Oliver Hartkopp @ 2016-11-22 17:29 UTC (permalink / raw)
  To: Andrey Konovalov, Marc Kleine-Budde, David S. Miller, linux-can,
	netdev, LKML
  Cc: Dmitry Vyukov, Alexander Potapenko, Kostya Serebryany,
	Eric Dumazet, syzkaller
In-Reply-To: <CAAeHK+xhwpVO=xHqEOyP-Tm6cx1+jDLDpYT0hpMwM2PCf7W3Jw@mail.gmail.com>

Hi Andrey,

thanks for the report.

Although I can't see the issue in the code ...

On 11/22/2016 10:22 AM, Andrey Konovalov wrote:

> ==================================================================
> BUG: KASAN: use-after-free in bcm_rx_thr_flush+0x284/0x2b0
> Read of size 1 at addr ffff88006c1faae5 by task a.out/3874
>
> page:ffffea0001b07e80 count:1 mapcount:0 mapping:          (null) index:0x0
> flags: 0x100000000000080(slab)
> page dumped because: kasan: bad access detected

(..)

>
> The buggy address belongs to the object at ffff88006c1faae0
>  which belongs to the cache kmalloc-32 of size 32

???

> The buggy address ffff88006c1faae5 is located 5 bytes inside
>  of 32-byte region [ffff88006c1faae0, ffff88006c1fab00)

(..)

> Memory state around the buggy address:
>  ffff88006c1fa980: fc fc fb fb fb fb fc fc fb fb fb fb fc fc fb fb
>  ffff88006c1faa00: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
>> ffff88006c1faa80: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
>                                                        ^
>  ffff88006c1fab00: fc fc fb fb fb fb fc fc 00 00 00 00 fc fc 00 00
>  ffff88006c1fab80: 00 00 fc fc fb fb fb fb fc fc fb fb fb fb fc fc
> ==================================================================

(should be some zero initialized memory here)

The relevant code of bcm_rx_do_flush() can be found here:

http://lxr.free-electrons.com/source/net/can/bcm.c#L589

static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
				  unsigned int index)
{
	struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;

	if ((op->last_frames) && (lcf->flags & RX_THR)) {  <<<----- !!!
		if (update)
			bcm_rx_changed(op, lcf);
		return 1;
	}
	return 0;
}


lcf->flags points into an array of struct canfd_frame at offset 5 which 
is allocated here:

http://lxr.free-electrons.com/source/net/can/bcm.c#L1105

	/* create and init array for received CAN frames */
	op->last_frames = kzalloc(msg_head->nframes * op->cfsiz,
				  GFP_KERNEL);

So why does KASAN complain about accessing some kind of 32 byte cache 
when it should point into a zero initialized allocated space?

I will write some other test cases with a similar setting of options to 
check if I can trigger the instability too.

Tnx & regards,
Oliver

^ permalink raw reply

* Re: net/can: use-after-free in bcm_rx_thr_flush
From: Andrey Konovalov @ 2016-11-22 17:37 UTC (permalink / raw)
  To: syzkaller
  Cc: Marc Kleine-Budde, David S. Miller, linux-can, netdev, LKML,
	Dmitry Vyukov, Alexander Potapenko, Kostya Serebryany,
	Eric Dumazet
In-Reply-To: <5abd6ebd-1249-71a9-da30-65d8371a2a36@hartkopp.net>

On Tue, Nov 22, 2016 at 6:29 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hi Andrey,
>
> thanks for the report.
>
> Although I can't see the issue in the code ...
>
> On 11/22/2016 10:22 AM, Andrey Konovalov wrote:
>
>> ==================================================================
>> BUG: KASAN: use-after-free in bcm_rx_thr_flush+0x284/0x2b0
>> Read of size 1 at addr ffff88006c1faae5 by task a.out/3874
>>
>> page:ffffea0001b07e80 count:1 mapcount:0 mapping:          (null)
>> index:0x0
>> flags: 0x100000000000080(slab)
>> page dumped because: kasan: bad access detected
>
>
> (..)
>
>>
>> The buggy address belongs to the object at ffff88006c1faae0
>>  which belongs to the cache kmalloc-32 of size 32
>
>
> ???
>
>> The buggy address ffff88006c1faae5 is located 5 bytes inside
>>  of 32-byte region [ffff88006c1faae0, ffff88006c1fab00)
>
>
> (..)
>
>> Memory state around the buggy address:
>>  ffff88006c1fa980: fc fc fb fb fb fb fc fc fb fb fb fb fc fc fb fb
>>  ffff88006c1faa00: fb fb fc fc fb fb fb fb fc fc fb fb fb fb fc fc
>>>
>>> ffff88006c1faa80: fb fb fb fb fc fc fb fb fb fb fc fc fb fb fb fb
>>
>>                                                        ^
>>  ffff88006c1fab00: fc fc fb fb fb fb fc fc 00 00 00 00 fc fc 00 00
>>  ffff88006c1fab80: 00 00 fc fc fb fb fb fb fc fc fb fb fb fb fc fc
>> ==================================================================
>
>
> (should be some zero initialized memory here)
>
> The relevant code of bcm_rx_do_flush() can be found here:
>
> http://lxr.free-electrons.com/source/net/can/bcm.c#L589
>
> static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
>                                   unsigned int index)
> {
>         struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;
>
>         if ((op->last_frames) && (lcf->flags & RX_THR)) {  <<<----- !!!
>                 if (update)
>                         bcm_rx_changed(op, lcf);
>                 return 1;
>         }
>         return 0;
> }
>
>
> lcf->flags points into an array of struct canfd_frame at offset 5 which is
> allocated here:
>
> http://lxr.free-electrons.com/source/net/can/bcm.c#L1105
>
>         /* create and init array for received CAN frames */
>         op->last_frames = kzalloc(msg_head->nframes * op->cfsiz,
>                                   GFP_KERNEL);
>
> So why does KASAN complain about accessing some kind of 32 byte cache when
> it should point into a zero initialized allocated space?

Hi Oliver,

My guess would be that this is an out-of-bounds access which doesn't
hit the redzone.
The free and alloc stack traces also look unrelated to the access.
Besides I have a bunch of related slab-out-of-bounds reports, see below.

Thanks for looking at this!

==================================================================
BUG: KASAN: slab-out-of-bounds in bcm_send_to_user+0x330/0x480
Read of size 16 at addr ffff88006de17338 by task syz-executor/30679

page:ffffea0001b78580 count:1 mapcount:0 mapping:          (null)
index:0xffff88006de16760 compound_mapcount: 0
flags: 0x500000000004080(slab|head)
page dumped because: kasan: bad access detected

CPU: 2 PID: 30679 Comm: syz-executor Not tainted 4.9.0-rc6+ #429
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffff88003cd277b0 ffffffff81b472e4 ffff88003cd27840 ffff88006de17338
 00000000000000fb 00000000000000fc ffff88003cd27830 ffffffff8150ad42
 0000000000000000 ffffffff81509f65 ffff88006aef9830 0000000000000282
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff81b472e4>] dump_stack+0xb3/0x10f lib/dump_stack.c:51
 [<     inline     >] describe_address mm/kasan/report.c:259
 [<ffffffff8150ad42>] kasan_report_error+0x122/0x560 mm/kasan/report.c:365
 [<ffffffff8150b536>] kasan_report+0x36/0x40 mm/kasan/report.c:387
 [<     inline     >] check_memory_region_inline mm/kasan/kasan.c:308
 [<ffffffff81509d2e>] check_memory_region+0x13e/0x1a0 mm/kasan/kasan.c:315
 [<ffffffff8150a223>] memcpy+0x23/0x50 mm/kasan/kasan.c:350
 [<ffffffff83574410>] bcm_send_to_user+0x330/0x480 net/can/bcm.c:325
 [<ffffffff8357478e>] bcm_rx_changed+0x22e/0x2a0 net/can/bcm.c:443
 [<     inline     >] bcm_rx_do_flush net/can/bcm.c:591
 [<ffffffff83577d1e>] bcm_rx_thr_flush+0x19e/0x2b0 net/can/bcm.c:612
 [<     inline     >] bcm_rx_setup net/can/bcm.c:1199
 [<ffffffff83578b36>] bcm_sendmsg+0xbb6/0x30e0 net/can/bcm.c:1351
 [<     inline     >] sock_sendmsg_nosec net/socket.c:621
 [<ffffffff82b7176c>] sock_sendmsg+0xcc/0x110 net/socket.c:631
 [<ffffffff82b73651>] ___sys_sendmsg+0x771/0x8b0 net/socket.c:1954
 [<ffffffff82b7563e>] __sys_sendmsg+0xce/0x170 net/socket.c:1988
 [<     inline     >] SYSC_sendmsg net/socket.c:1999
 [<ffffffff82b7570d>] SyS_sendmsg+0x2d/0x50 net/socket.c:1995
 [<ffffffff83fc4301>] entry_SYSCALL_64_fastpath+0x1f/0xc2

The buggy address belongs to the object at ffff88006de17320
 which belongs to the cache kmalloc-32 of size 32
The buggy address ffff88006de17338 is located 24 bytes inside
 of 32-byte region [ffff88006de17320, ffff88006de17340)

Freed by task 0:
 [<ffffffff8107e236>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 [<ffffffff81509e56>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [<     inline     >] set_track mm/kasan/kasan.c:507
 [<ffffffff8150a6b3>] kasan_slab_free+0x73/0xc0 mm/kasan/kasan.c:571
 [<     inline     >] slab_free_hook mm/slub.c:1352
 [<     inline     >] slab_free_freelist_hook mm/slub.c:1374
 [<     inline     >] slab_free mm/slub.c:2951
 [<ffffffff81506b98>] kfree+0xe8/0x2b0 mm/slub.c:3871
 [<ffffffff819dd8c1>] selinux_cred_free+0x51/0x80 security/selinux/hooks.c:3725
 [<ffffffff819ce358>] security_cred_free+0x48/0x80 security/security.c:907
 [<ffffffff8117e27d>] put_cred_rcu+0xed/0x390 kernel/cred.c:116
 [<     inline     >] __rcu_reclaim kernel/rcu/rcu.h:118
 [<     inline     >] rcu_do_batch kernel/rcu/tree.c:2776
 [<     inline     >] invoke_rcu_callbacks kernel/rcu/tree.c:3040
 [<     inline     >] __rcu_process_callbacks kernel/rcu/tree.c:3007
 [<ffffffff8125dfe0>] rcu_process_callbacks+0xa40/0x1190 kernel/rcu/tree.c:3024
 [<ffffffff83fc70af>] __do_softirq+0x23f/0x8e5 kernel/softirq.c:284

Allocated by task 4074:
 [<ffffffff8107e236>] save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
 [<ffffffff81509e56>] save_stack+0x46/0xd0 mm/kasan/kasan.c:495
 [<     inline     >] set_track mm/kasan/kasan.c:507
 [<ffffffff8150a0cb>] kasan_kmalloc+0xab/0xe0 mm/kasan/kasan.c:598
 [<ffffffff8150a632>] kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:537
 [<     inline     >] slab_post_alloc_hook mm/slab.h:417
 [<     inline     >] slab_alloc_node mm/slub.c:2708
 [<     inline     >] slab_alloc mm/slub.c:2716
 [<ffffffff815090ef>] __kmalloc_track_caller+0xcf/0x2a0 mm/slub.c:4240
 [<ffffffff8146bf84>] kmemdup+0x24/0x50 mm/util.c:113
 [<ffffffff819dcbe9>] selinux_cred_prepare+0x49/0xb0
security/selinux/hooks.c:3739
 [<ffffffff819ce40d>] security_prepare_creds+0x7d/0xb0 security/security.c:912
 [<ffffffff8117fab3>] prepare_creds+0x243/0x340 kernel/cred.c:277
 [<ffffffff811876a4>] set_current_groups+0x14/0x50 kernel/groups.c:155
 [<     inline     >] SYSC_setgroups kernel/groups.c:221
 [<ffffffff8118807f>] SyS_setgroups+0x17f/0x1d0 kernel/groups.c:202
 [<ffffffff83fc4301>] entry_SYSCALL_64_fastpath+0x1f/0xc2

Memory state around the buggy address:
 ffff88006de17200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff88006de17280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88006de17300: fc fc fc fc 00 00 00 00 fc fc fc fc fc fc fc fc
                                           ^
 ffff88006de17380: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 ffff88006de17400: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================


>
> I will write some other test cases with a similar setting of options to
> check if I can trigger the instability too.
>
> Tnx & regards,
> Oliver
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply

* Re: net/can: use-after-free in bcm_rx_thr_flush
From: Oliver Hartkopp @ 2016-11-23  6:06 UTC (permalink / raw)
  To: Andrey Konovalov, syzkaller
  Cc: Marc Kleine-Budde, David S. Miller, linux-can, netdev, LKML,
	Dmitry Vyukov, Alexander Potapenko, Kostya Serebryany,
	Eric Dumazet
In-Reply-To: <CAAeHK+x+SwBFj3iRfNN0S-5=kPXLdTgGZaoBAT+L0QpdU+0-vw@mail.gmail.com>

On 11/22/2016 06:37 PM, Andrey Konovalov wrote:
> On Tue, Nov 22, 2016 at 6:29 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>> Hi Andrey,
>>
>> thanks for the report.
>>
>> Although I can't see the issue in the code ...
>>

Oh, I can see it now m(

Will send a patch today.

Many thanks,
Oliver

^ permalink raw reply

* [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Magnus Damm, Rob Herring,
	Mark Rutland, Ramesh Shanmugasundaram, linux-renesas-soc,
	devicetree, linux-arm-kernel, linux-can, Chris Paterson

This patch series adds CAN and CAN FD support to the r8a7796.

Based on renesas-devel-20161122-v4.9-rc6.

Chris Paterson (3):
  arm64: dts: r8a7796: Add CAN external clock support
  arm64: dts: r8a7796: Add CAN support
  arm64: dts: r8a7796: Add CAN FD support

 .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
 .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61 ++++++++++++++++++++++
 3 files changed, 75 insertions(+), 10 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 2/3] arm64: dts: r8a7796: Add CAN support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Magnus Damm, Rob Herring,
	Mark Rutland, Ramesh Shanmugasundaram, linux-renesas-soc,
	devicetree, linux-arm-kernel, linux-can, Chris Paterson

Adds CAN controller nodes for r8a7796.

Based on a patch for r8a7795 by Ramesh Shanmugasundaram.

Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
---
 .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++++----
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 30 ++++++++++++++++++++++
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index 8d40ab2..06bb7cc 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -10,6 +10,7 @@ Required properties:
 	      "renesas,can-r8a7793" if CAN controller is a part of R8A7793 SoC.
 	      "renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
 	      "renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
+	      "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
 	      "renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
 	      "renesas,rcar-gen2-can" for a generic R-Car Gen2 compatible device.
 	      "renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible device.
@@ -24,11 +25,12 @@ Required properties:
 - pinctrl-0: pin control group to be used for this controller.
 - pinctrl-names: must be "default".
 
-Required properties for "renesas,can-r8a7795" compatible:
-In R8A7795 SoC, "clkp2" can be CANFD clock. This is a div6 clock and can be
-used by both CAN and CAN FD controller at the same time. It needs to be scaled
-to maximum frequency if any of these controllers use it. This is done using
-the below properties.
+Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
+compatible:
+In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
+and can be used by both CAN and CAN FD controller at the same time. It needs to
+be scaled to maximum frequency if any of these controllers use it. This is done
+using the below properties:
 
 - assigned-clocks: phandle of clkp2(CANFD) clock.
 - assigned-clock-rates: maximum frequency of this clock.
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 91d716e..95857fc 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -362,6 +362,36 @@
 			status = "disabled";
 		};
 
+		can0: can@e6c30000 {
+			compatible = "renesas,can-r8a7796",
+				     "renesas,rcar-gen3-can";
+			reg = <0 0xe6c30000 0 0x1000>;
+			interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 916>,
+			       <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+			       <&can_clk>;
+			clock-names = "clkp1", "clkp2", "can_clk";
+			assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+			assigned-clock-rates = <40000000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
+		can1: can@e6c38000 {
+			compatible = "renesas,can-r8a7796",
+				     "renesas,rcar-gen3-can";
+			reg = <0 0xe6c38000 0 0x1000>;
+			interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 915>,
+			       <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+			       <&can_clk>;
+			clock-names = "clkp1", "clkp2", "can_clk";
+			assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+			assigned-clock-rates = <40000000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			status = "disabled";
+		};
+
 		scif2: serial@e6e88000 {
 			compatible = "renesas,scif-r8a7796",
 				     "renesas,rcar-gen3-scif", "renesas,scif";
-- 
1.9.1


^ permalink raw reply related

* [PATCH 3/3] arm64: dts: r8a7796: Add CAN FD support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Magnus Damm, Rob Herring,
	Mark Rutland, Ramesh Shanmugasundaram, linux-renesas-soc,
	devicetree, linux-arm-kernel, linux-can, Chris Paterson

Adds CAN FD controller node for r8a7796.

Based on a patch for r8a7795 by Ramesh Shanmugasundaram.

Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
---
 .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 ++++++-----
 arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 24 ++++++++++++++++++++++
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 22a6f10..788f273 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -5,6 +5,7 @@ Required properties:
 - compatible: Must contain one or more of the following:
   - "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
   - "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible controller.
+  - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3) compatible controller.
 
   When compatible with the generic version, nodes must list the
   SoC-specific version corresponding to the platform first, followed by the
@@ -23,11 +24,12 @@ The name of the child nodes are "channel0" and "channel1" respectively. Each
 child node supports the "status" property only, which is used to
 enable/disable the respective channel.
 
-Required properties for "renesas,r8a7795-canfd" compatible:
-In R8A7795 SoC, canfd clock is a div6 clock and can be used by both CAN
-and CAN FD controller at the same time. It needs to be scaled to maximum
-frequency if any of these controllers use it. This is done using the
-below properties.
+Required properties for "renesas,r8a7795-canfd" and "renesas,r8a7796-canfd"
+compatible:
+In R8A7795 and R8A7796 SoCs, canfd clock is a div6 clock and can be used by both
+CAN and CAN FD controller at the same time. It needs to be scaled to maximum
+frequency if any of these controllers use it. This is done using the below
+properties:
 
 - assigned-clocks: phandle of canfd clock.
 - assigned-clock-rates: maximum frequency of this clock.
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 95857fc..c069d77 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -392,6 +392,30 @@
 			status = "disabled";
 		};
 
+		canfd: can@e66c0000 {
+			compatible = "renesas,r8a7796-canfd",
+				     "renesas,rcar-gen3-canfd";
+			reg = <0 0xe66c0000 0 0x8000>;
+			interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+				   <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 914>,
+			       <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+			       <&can_clk>;
+			clock-names = "fck", "canfd", "can_clk";
+			assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+			assigned-clock-rates = <40000000>;
+			power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+			status = "disabled";
+
+			channel0 {
+				status = "disabled";
+			};
+
+			channel1 {
+				status = "disabled";
+			};
+		};
+
 		scif2: serial@e6e88000 {
 			compatible = "renesas,scif-r8a7796",
 				     "renesas,rcar-gen3-scif", "renesas,scif";
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH 2/3] arm64: dts: r8a7796: Add CAN support
From: Geert Uytterhoeven @ 2016-11-23 13:10 UTC (permalink / raw)
  To: Chris Paterson
  Cc: Simon Horman, Wolfgang Grandegger, Marc Kleine-Budde, Magnus Damm,
	Rob Herring, Mark Rutland, Ramesh Shanmugasundaram, Linux-Renesas,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-can
In-Reply-To: <1479903279-1950-1-git-send-email-chris.paterson2@renesas.com>

On Wed, Nov 23, 2016 at 1:14 PM, Chris Paterson
<chris.paterson2@renesas.com> wrote:
> Adds CAN controller nodes for r8a7796.
>
> Based on a patch for r8a7795 by Ramesh Shanmugasundaram.
>
> Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Marc Kleine-Budde @ 2016-11-23 13:18 UTC (permalink / raw)
  To: Chris Paterson, Simon Horman
  Cc: Wolfgang Grandegger, Magnus Damm, Rob Herring, Mark Rutland,
	Ramesh Shanmugasundaram, linux-renesas-soc, devicetree,
	linux-arm-kernel, linux-can
In-Reply-To: <1479903243-1860-1-git-send-email-chris.paterson2@renesas.com>


[-- Attachment #1.1: Type: text/plain, Size: 987 bytes --]

On 11/23/2016 01:14 PM, Chris Paterson wrote:
> This patch series adds CAN and CAN FD support to the r8a7796.
> 
> Based on renesas-devel-20161122-v4.9-rc6.
> 
> Chris Paterson (3):
>   arm64: dts: r8a7796: Add CAN external clock support
>   arm64: dts: r8a7796: Add CAN support
>   arm64: dts: r8a7796: Add CAN FD support
> 
>  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
>  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
>  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61 ++++++++++++++++++++++
>  3 files changed, 75 insertions(+), 10 deletions(-)

For all three:

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

Who takes this series?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH 3/3] arm64: dts: r8a7796: Add CAN FD support
From: Geert Uytterhoeven @ 2016-11-23 13:19 UTC (permalink / raw)
  To: Chris Paterson
  Cc: Simon Horman, Wolfgang Grandegger, Marc Kleine-Budde, Magnus Damm,
	Rob Herring, Mark Rutland, Ramesh Shanmugasundaram, Linux-Renesas,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-can
In-Reply-To: <1479903288-2009-1-git-send-email-chris.paterson2@renesas.com>

On Wed, Nov 23, 2016 at 1:14 PM, Chris Paterson
<chris.paterson2@renesas.com> wrote:
> Adds CAN FD controller node for r8a7796.
>
> Based on a patch for r8a7795 by Ramesh Shanmugasundaram.
>
> Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] can: bcm: fix support for CAN FD frames
From: Oliver Hartkopp @ 2016-11-23 13:33 UTC (permalink / raw)
  To: linux-can, Andrey Konovalov, netdev; +Cc: Oliver Hartkopp

Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.

As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.

This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.

Reference: http://marc.info/?l=linux-netdev&m=147980658909653

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25..436a753 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
 		     (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
 		     (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
 
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
 
 MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
 	u32 count;
 	u32 nframes;
 	u32 currframe;
-	struct canfd_frame *frames;
-	struct canfd_frame *last_frames;
+	/* void pointers to arrays of struct can[fd]_frame */
+	void *frames;
+	void *last_frames;
 	struct canfd_frame sframe;
 	struct canfd_frame last_sframe;
 	struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 
 	if (op->flags & RX_FILTER_ID) {
 		/* the easiest case */
-		bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+		bcm_rx_update_and_send(op, op->last_frames, rxframe);
 		goto rx_starttimer;
 	}
 
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 
 		if (msg_head->nframes) {
 			/* update CAN frames content */
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0)
 				return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		}
 
 		if (msg_head->nframes) {
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0) {
 				if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 	/* check flags */
 
 	if (op->flags & RX_RTR_FRAME) {
+		struct canfd_frame *frame0 = op->frames;
 
 		/* no timers in RTR-mode */
 		hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		 * prevent a full-load-loopback-test ... ;-]
 		 */
 		if ((op->flags & TX_CP_CAN_ID) ||
-		    (op->frames[0].can_id == op->can_id))
-			op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+		    (frame0->can_id == op->can_id))
+			frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
 
 	} else {
 		if (op->flags & SETTIMER) {
-- 
2.10.2


^ permalink raw reply related

* [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
From: Geert Uytterhoeven @ 2016-11-23 13:44 UTC (permalink / raw)
  To: Wolfgang Grandegger, Marc Kleine-Budde, Ramesh Shanmugasundaram,
	Chris Paterson
  Cc: linux-can, netdev, devicetree, linux-renesas-soc,
	Geert Uytterhoeven

According to both DTS (example and actual files), and Linux driver code,
the first interrupt specifier should be the Channel interrupt, while the
second interrupt specifier should be the Global interrupt.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 22a6f10bab057f46..58c27cd839e3a2ac 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -11,7 +11,7 @@ Required properties:
   family-specific and/or generic versions.
 
 - reg: physical base address and size of the R-Car CAN FD register map.
-- interrupts: interrupt specifier for the Global & Channel interrupts
+- interrupts: interrupt specifiers for the Channel & Global interrupts
 - clocks: phandles and clock specifiers for 3 clock inputs.
 - clock-names: 3 clock input name strings: "fck", "canfd", "can_clk".
 - pinctrl-0: pin control group to be used for this controller.
-- 
1.9.1


^ permalink raw reply related

* RE: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
From: Ramesh Shanmugasundaram @ 2016-11-23 14:04 UTC (permalink / raw)
  To: Geert Uytterhoeven, Wolfgang Grandegger, Marc Kleine-Budde,
	Chris Paterson
  Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <1479908686-14028-1-git-send-email-geert+renesas@glider.be>

Hi Geert,

> Subject: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
> 
> According to both DTS (example and actual files), and Linux driver code,
> the first interrupt specifier should be the Channel interrupt, while the
> second interrupt specifier should be the Global interrupt.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> index 22a6f10bab057f46..58c27cd839e3a2ac 100644
> --- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> +++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> @@ -11,7 +11,7 @@ Required properties:
>    family-specific and/or generic versions.
> 
>  - reg: physical base address and size of the R-Car CAN FD register map.
> -- interrupts: interrupt specifier for the Global & Channel interrupts
> +- interrupts: interrupt specifiers for the Channel & Global interrupts

Thanks for correcting it.
Acked-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>

Thanks,
Ramesh

^ permalink raw reply

* Re: [PATCH] can: bcm: fix support for CAN FD frames
From: Andrey Konovalov @ 2016-11-23 14:15 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can, netdev
In-Reply-To: <20161123133325.1812-1-socketcan@hartkopp.net>

On Wed, Nov 23, 2016 at 2:33 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
> CAN broadcast manager supports CAN and CAN FD data frames.
>
> As these data frames are embedded in struct can[fd]_frames which have a
> different length the access to the provided array of CAN frames became
> dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
> CAN frames the new offset calculation based on op->cfsiz was accidently applied
> to CAN FD frame element lengths.
>
> This fix makes the pointer to the arrays of the different CAN frame types a
> void pointer so that the offset calculation in bytes accesses the correct CAN
> frame elements.
>
> Reference: http://marc.info/?l=linux-netdev&m=147980658909653
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
>  net/can/bcm.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 8af9d25..436a753 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -77,7 +77,7 @@
>                      (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
>                      (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
>
> -#define CAN_BCM_VERSION "20160617"
> +#define CAN_BCM_VERSION "20161123"
>
>  MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
>  MODULE_LICENSE("Dual BSD/GPL");
> @@ -109,8 +109,9 @@ struct bcm_op {
>         u32 count;
>         u32 nframes;
>         u32 currframe;
> -       struct canfd_frame *frames;
> -       struct canfd_frame *last_frames;
> +       /* void pointers to arrays of struct can[fd]_frame */
> +       void *frames;
> +       void *last_frames;
>         struct canfd_frame sframe;
>         struct canfd_frame last_sframe;
>         struct sock *sk;
> @@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
>
>         if (op->flags & RX_FILTER_ID) {
>                 /* the easiest case */
> -               bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
> +               bcm_rx_update_and_send(op, op->last_frames, rxframe);
>                 goto rx_starttimer;
>         }
>
> @@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>
>                 if (msg_head->nframes) {
>                         /* update CAN frames content */
> -                       err = memcpy_from_msg((u8 *)op->frames, msg,
> +                       err = memcpy_from_msg(op->frames, msg,
>                                               msg_head->nframes * op->cfsiz);
>                         if (err < 0)
>                                 return err;
> @@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>                 }
>
>                 if (msg_head->nframes) {
> -                       err = memcpy_from_msg((u8 *)op->frames, msg,
> +                       err = memcpy_from_msg(op->frames, msg,
>                                               msg_head->nframes * op->cfsiz);
>                         if (err < 0) {
>                                 if (op->frames != &op->sframe)
> @@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>         /* check flags */
>
>         if (op->flags & RX_RTR_FRAME) {
> +               struct canfd_frame *frame0 = op->frames;
>
>                 /* no timers in RTR-mode */
>                 hrtimer_cancel(&op->thrtimer);
> @@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>                  * prevent a full-load-loopback-test ... ;-]
>                  */
>                 if ((op->flags & TX_CP_CAN_ID) ||
> -                   (op->frames[0].can_id == op->can_id))
> -                       op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
> +                   (frame0->can_id == op->can_id))
> +                       frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
>
>         } else {
>                 if (op->flags & SETTIMER) {
> --
> 2.10.2
>

Tested-by: Andrey Konovalov <andreyknvl@google.com>

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-23 14:29 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
	linux-renesas-soc, Rob Herring, linux-arm-kernel,
	Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <f358442c-8373-328c-1270-709498c133f5@pengutronix.de>

On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > This patch series adds CAN and CAN FD support to the r8a7796.
> > 
> > Based on renesas-devel-20161122-v4.9-rc6.
> > 
> > Chris Paterson (3):
> >   arm64: dts: r8a7796: Add CAN external clock support
> >   arm64: dts: r8a7796: Add CAN support
> >   arm64: dts: r8a7796: Add CAN FD support
> > 
> >  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
> >  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
> >  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61 ++++++++++++++++++++++
> >  3 files changed, 75 insertions(+), 10 deletions(-)
> 
> For all three:
> 
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> Who takes this series?

I would like to see these patches split up so that the
.../devicetree/bindings/ portions can go through you whole
the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.

Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
some consideration given to what effect enabling memory above 4Gb
(64bit addressing) would have.

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Marc Kleine-Budde @ 2016-11-23 14:31 UTC (permalink / raw)
  To: Simon Horman
  Cc: Mark Rutland, devicetree, Chris Paterson, Magnus Damm, linux-can,
	linux-renesas-soc, Rob Herring, linux-arm-kernel,
	Ramesh Shanmugasundaram, Wolfgang Grandegger
In-Reply-To: <20161123142938.GF9057@verge.net.au>


[-- Attachment #1.1.1: Type: text/plain, Size: 1519 bytes --]

On 11/23/2016 03:29 PM, Simon Horman wrote:
> On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
>> On 11/23/2016 01:14 PM, Chris Paterson wrote:
>>> This patch series adds CAN and CAN FD support to the r8a7796.
>>>
>>> Based on renesas-devel-20161122-v4.9-rc6.
>>>
>>> Chris Paterson (3):
>>>   arm64: dts: r8a7796: Add CAN external clock support
>>>   arm64: dts: r8a7796: Add CAN support
>>>   arm64: dts: r8a7796: Add CAN FD support
>>>
>>>  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
>>>  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
>>>  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61 ++++++++++++++++++++++
>>>  3 files changed, 75 insertions(+), 10 deletions(-)
>>
>> For all three:
>>
>> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>
>> Who takes this series?
> 
> I would like to see these patches split up so that the
> .../devicetree/bindings/ portions can go through you whole
> the arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.

Ok

> Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> some consideration given to what effect enabling memory above 4Gb
> (64bit addressing) would have.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* pull-request: can 2016-11-23
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request for net/master.

The patch by Oliver Hartkopp for the broadcast manager (bcm) fixes the CAN-FD
support, which may cause an out-of-bounds access otherwise.

regards,
Marc
---

The following changes since commit c9b8af1330198ae241cd545e1f040019010d44d9:

  flow_dissect: call init_default_flow_dissectors() earlier (2016-11-22 14:44:01 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.9-20161123

for you to fetch changes up to 5499a6b22e5508b921c447757685b0a5e40a07ed:

  can: bcm: fix support for CAN FD frames (2016-11-23 15:22:18 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.9-20161123

----------------------------------------------------------------
Oliver Hartkopp (1):
      can: bcm: fix support for CAN FD frames

 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

^ permalink raw reply

* [PATCH] can: bcm: fix support for CAN FD frames
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Oliver Hartkopp, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20161123143430.24985-1-mkl@pengutronix.de>

From: Oliver Hartkopp <socketcan@hartkopp.net>

Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.

As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.

This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.

Reference: http://marc.info/?l=linux-netdev&m=147980658909653

Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 net/can/bcm.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25ff988..436a7537e6a9 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
 		     (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
 		     (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
 
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
 
 MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
 	u32 count;
 	u32 nframes;
 	u32 currframe;
-	struct canfd_frame *frames;
-	struct canfd_frame *last_frames;
+	/* void pointers to arrays of struct can[fd]_frame */
+	void *frames;
+	void *last_frames;
 	struct canfd_frame sframe;
 	struct canfd_frame last_sframe;
 	struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 
 	if (op->flags & RX_FILTER_ID) {
 		/* the easiest case */
-		bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+		bcm_rx_update_and_send(op, op->last_frames, rxframe);
 		goto rx_starttimer;
 	}
 
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 
 		if (msg_head->nframes) {
 			/* update CAN frames content */
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0)
 				return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		}
 
 		if (msg_head->nframes) {
-			err = memcpy_from_msg((u8 *)op->frames, msg,
+			err = memcpy_from_msg(op->frames, msg,
 					      msg_head->nframes * op->cfsiz);
 			if (err < 0) {
 				if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 	/* check flags */
 
 	if (op->flags & RX_RTR_FRAME) {
+		struct canfd_frame *frame0 = op->frames;
 
 		/* no timers in RTR-mode */
 		hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		 * prevent a full-load-loopback-test ... ;-]
 		 */
 		if ((op->flags & TX_CP_CAN_ID) ||
-		    (op->frames[0].can_id == op->can_id))
-			op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+		    (frame0->can_id == op->can_id))
+			frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
 
 	} else {
 		if (op->flags & SETTIMER) {
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH] can: bcm: fix support for CAN FD frames
From: Marc Kleine-Budde @ 2016-11-23 14:35 UTC (permalink / raw)
  To: Oliver Hartkopp, linux-can, Andrey Konovalov, netdev
In-Reply-To: <20161123133325.1812-1-socketcan@hartkopp.net>


[-- Attachment #1.1: Type: text/plain, Size: 1260 bytes --]

On 11/23/2016 02:33 PM, Oliver Hartkopp wrote:
> Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
> CAN broadcast manager supports CAN and CAN FD data frames.
> 
> As these data frames are embedded in struct can[fd]_frames which have a
> different length the access to the provided array of CAN frames became
> dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
> CAN frames the new offset calculation based on op->cfsiz was accidently applied
> to CAN FD frame element lengths.
> 
> This fix makes the pointer to the arrays of the different CAN frame types a
> void pointer so that the offset calculation in bytes accesses the correct CAN
> frame elements.
> 
> Reference: http://marc.info/?l=linux-netdev&m=147980658909653
> 
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>

Added to can and send a pull request to David.

Thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* RE: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-24 10:05 UTC (permalink / raw)
  To: Simon Horman, Marc Kleine-Budde
  Cc: Wolfgang Grandegger, Magnus Damm, Rob Herring, Mark Rutland,
	Ramesh Shanmugasundaram, linux-renesas-soc@vger.kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-can@vger.kernel.org
In-Reply-To: <20161123142938.GF9057@verge.net.au>

Hello Simon,

From: Simon Horman [mailto:horms@verge.net.au]
Sent: 23 November 2016 14:30
> On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > This patch series adds CAN and CAN FD support to the r8a7796.
> > >
> > > Based on renesas-devel-20161122-v4.9-rc6.
> > >
> > > Chris Paterson (3):
> > >   arm64: dts: r8a7796: Add CAN external clock support
> > >   arm64: dts: r8a7796: Add CAN support
> > >   arm64: dts: r8a7796: Add CAN FD support
> > >
> > >  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
> > >  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
> > >  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61
> ++++++++++++++++++++++
> > >  3 files changed, 75 insertions(+), 10 deletions(-)
> >
> > For all three:
> >
> > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >
> > Who takes this series?
> 
> I would like to see these patches split up so that the .../devicetree/bindings/
> portions can go through you whole the arch/arm64/boot/dts/renesas/
> portions go thorugh my renesas tree.

Okay, will do.

> 
> Regarding the arch/arm64/boot/dts/renesas/ portion, I would like some
> consideration given to what effect enabling memory above 4Gb (64bit
> addressing) would have.

Can you give me some guidance here? I'm not sure what you're referring to. As far as I know the DT reg definition here is 64-bit, or are you referring to DMA usage? If the later, neither CAN driver uses DMA.

Kind regards, Chris

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-24 10:17 UTC (permalink / raw)
  To: Chris Paterson
  Cc: Mark Rutland, devicetree@vger.kernel.org, Magnus Damm,
	linux-can@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	Rob Herring, Marc Kleine-Budde,
	linux-arm-kernel@lists.infradead.org, Ramesh Shanmugasundaram,
	Wolfgang Grandegger
In-Reply-To: <HK2PR0601MB132929329C3574A0B4D9A0EAB7B60@HK2PR0601MB1329.apcprd06.prod.outlook.com>

Hi Chris,

On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> Hello Simon,
> 
> From: Simon Horman [mailto:horms@verge.net.au]
> Sent: 23 November 2016 14:30
> > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > > This patch series adds CAN and CAN FD support to the r8a7796.
> > > >
> > > > Based on renesas-devel-20161122-v4.9-rc6.
> > > >
> > > > Chris Paterson (3):
> > > >   arm64: dts: r8a7796: Add CAN external clock support
> > > >   arm64: dts: r8a7796: Add CAN support
> > > >   arm64: dts: r8a7796: Add CAN FD support
> > > >
> > > >  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
> > > >  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
> > > >  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61
> > ++++++++++++++++++++++
> > > >  3 files changed, 75 insertions(+), 10 deletions(-)
> > >
> > > For all three:
> > >
> > > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > >
> > > Who takes this series?
> > 
> > I would like to see these patches split up so that the .../devicetree/bindings/
> > portions can go through you whole the arch/arm64/boot/dts/renesas/
> > portions go thorugh my renesas tree.
> 
> Okay, will do.

Thanks.

> > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like some
> > consideration given to what effect enabling memory above 4Gb (64bit
> > addressing) would have.
> 
> Can you give me some guidance here? I'm not sure what you're referring
> to. As far as I know the DT reg definition here is 64-bit, or are you
> referring to DMA usage? If the later, neither CAN driver uses DMA.

Sorry for not being clearer.

What I would like to know is if there are any problems in the CAN driver
or hardware that would prevent it from functioning with memory that
requires 64bit addressing present.

If the CAN hardware cannot use DMA then DMA doesn't need to be taken into
account. But if it DMA could be enabled in future for CAN, for example
after some driver enhancements, then it would be good to know if 64bit
memory can be supported - if not it would imply DMA cannot be enabled.

As for non-DMA mode, will this function if memory above 4G is present?
If not then in theory such memory couldn't be enabled if the CAN driver
is enabled. This is my main concern.

Does the above help?

^ permalink raw reply

* RE: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-24 14:25 UTC (permalink / raw)
  To: Simon Horman
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Magnus Damm, Rob Herring,
	Mark Rutland, Ramesh Shanmugasundaram,
	linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-can@vger.kernel.org
In-Reply-To: <20161124101733.GA18027@verge.net.au>

Hello Simon,

From: Simon Horman [mailto:horms@verge.net.au]
Sent: 24 November 2016 10:18
> Hi Chris,
> 
> On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> > Hello Simon,
> >
> > From: Simon Horman [mailto:horms@verge.net.au]
> > Sent: 23 November 2016 14:30
> > > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > > On 11/23/2016 01:14 PM, Chris Paterson wrote:
> > > > > This patch series adds CAN and CAN FD support to the r8a7796.
> > > > >
> > > > > Based on renesas-devel-20161122-v4.9-rc6.
> > > > >
> > > > > Chris Paterson (3):
> > > > >   arm64: dts: r8a7796: Add CAN external clock support
> > > > >   arm64: dts: r8a7796: Add CAN support
> > > > >   arm64: dts: r8a7796: Add CAN FD support
> > > > >
> > > > >  .../devicetree/bindings/net/can/rcar_can.txt       | 12 +++--
> > > > >  .../devicetree/bindings/net/can/rcar_canfd.txt     | 12 +++--
> > > > >  arch/arm64/boot/dts/renesas/r8a7796.dtsi           | 61
> > > ++++++++++++++++++++++
> > > > >  3 files changed, 75 insertions(+), 10 deletions(-)
> > > >
> > > > For all three:
> > > >
> > > > Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> > > >
> > > > Who takes this series?
> > >
> > > I would like to see these patches split up so that the
> > > .../devicetree/bindings/ portions can go through you whole the
> > > arch/arm64/boot/dts/renesas/ portions go thorugh my renesas tree.
> >
> > Okay, will do.
> 
> Thanks.
> 
> > > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> > > some consideration given to what effect enabling memory above 4Gb
> > > (64bit
> > > addressing) would have.
> >
> > Can you give me some guidance here? I'm not sure what you're referring
> > to. As far as I know the DT reg definition here is 64-bit, or are you
> > referring to DMA usage? If the later, neither CAN driver uses DMA.
> 
> Sorry for not being clearer.
> 
> What I would like to know is if there are any problems in the CAN driver or
> hardware that would prevent it from functioning with memory that requires
> 64bit addressing present.
> 
> If the CAN hardware cannot use DMA then DMA doesn't need to be taken
> into account. But if it DMA could be enabled in future for CAN, for example
> after some driver enhancements, then it would be good to know if 64bit
> memory can be supported - if not it would imply DMA cannot be enabled.

Thank you for the clarification.

The CAN interface for r8a7795/6 does not support DMA.

With CAN FD there is currently a H/W issue that means DMA is unusable. Potentially this issue could be fixed in the future and DMA support could be added to the driver. If this happens I can see no reason why the CAN FD IP wouldn't be able to handle DMA transfers when using 64bit addressing.

> 
> As for non-DMA mode, will this function if memory above 4G is present?
> If not then in theory such memory couldn't be enabled if the CAN driver is
> enabled. This is my main concern.

Yes, it functions fine. We have already tested this with the Renesas v3.3.2 BSP with >4Gb memory.

If this is explanation okay for you, I'll proceed with a v2 splitting off the DT bindings documentation.

Kind regards, Chris

> 
> Does the above help?

^ permalink raw reply

* Re: [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Simon Horman @ 2016-11-24 14:32 UTC (permalink / raw)
  To: Chris Paterson
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Magnus Damm, Rob Herring,
	Mark Rutland, Ramesh Shanmugasundaram,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <HK2PR0601MB1329C747CA5C6B5222C02C3FB7B60-5BHi1SMfQIfsvBovKiDY8NK/flDYrvD0nBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>

Hi Chris,

On Thu, Nov 24, 2016 at 02:25:06PM +0000, Chris Paterson wrote:
> Hello Simon,
> 
> From: Simon Horman [mailto:horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org]
> Sent: 24 November 2016 10:18
> > Hi Chris,
> > 
> > On Thu, Nov 24, 2016 at 10:05:08AM +0000, Chris Paterson wrote:
> > > Hello Simon,
> > >
> > > From: Simon Horman [mailto:horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org]
> > > Sent: 23 November 2016 14:30
> > > > On Wed, Nov 23, 2016 at 02:18:13PM +0100, Marc Kleine-Budde wrote:
> > > > > On 11/23/2016 01:14 PM, Chris Paterson wrote:

...

> > > > Regarding the arch/arm64/boot/dts/renesas/ portion, I would like
> > > > some consideration given to what effect enabling memory above 4Gb
> > > > (64bit
> > > > addressing) would have.
> > >
> > > Can you give me some guidance here? I'm not sure what you're referring
> > > to. As far as I know the DT reg definition here is 64-bit, or are you
> > > referring to DMA usage? If the later, neither CAN driver uses DMA.
> > 
> > Sorry for not being clearer.
> > 
> > What I would like to know is if there are any problems in the CAN driver or
> > hardware that would prevent it from functioning with memory that requires
> > 64bit addressing present.
> > 
> > If the CAN hardware cannot use DMA then DMA doesn't need to be taken
> > into account. But if it DMA could be enabled in future for CAN, for example
> > after some driver enhancements, then it would be good to know if 64bit
> > memory can be supported - if not it would imply DMA cannot be enabled.
> 
> Thank you for the clarification.
> 
> The CAN interface for r8a7795/6 does not support DMA.
> 
> With CAN FD there is currently a H/W issue that means DMA is unusable.
> Potentially this issue could be fixed in the future and DMA support could
> be added to the driver. If this happens I can see no reason why the CAN
> FD IP wouldn't be able to handle DMA transfers when using 64bit
> addressing.
> 
> > 
> > As for non-DMA mode, will this function if memory above 4G is present?
> > If not then in theory such memory couldn't be enabled if the CAN driver
> > is enabled. This is my main concern.
> 
> Yes, it functions fine. We have already tested this with the Renesas
> v3.3.2 BSP with >4Gb memory.
> 
> If this is explanation okay for you, I'll proceed with a v2 splitting off
> the DT bindings documentation.

Thanks for the explanation. I think it would be good to proceed with a v2.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox