10 Jan, 2019

1 commit

  • [ Upstream commit d63967e475ae10f286dbd35e189cb241e0b1f284 ]

    Since capi_ioctl() copies 64 bytes after calling
    capi20_get_manufacturer() we need to ensure to not leak
    information to user.

    BUG: KMSAN: kernel-infoleak in _copy_to_user+0x16b/0x1f0 lib/usercopy.c:32
    CPU: 0 PID: 11245 Comm: syz-executor633 Not tainted 4.20.0-rc7+ #2
    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+0x173/0x1d0 lib/dump_stack.c:113
    kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613
    kmsan_internal_check_memory+0x9d4/0xb00 mm/kmsan/kmsan.c:704
    kmsan_copy_to_user+0xab/0xc0 mm/kmsan/kmsan_hooks.c:601
    _copy_to_user+0x16b/0x1f0 lib/usercopy.c:32
    capi_ioctl include/linux/uaccess.h:177 [inline]
    capi_unlocked_ioctl+0x1a0b/0x1bf0 drivers/isdn/capi/capi.c:939
    do_vfs_ioctl+0xebd/0x2bf0 fs/ioctl.c:46
    ksys_ioctl fs/ioctl.c:713 [inline]
    __do_sys_ioctl fs/ioctl.c:720 [inline]
    __se_sys_ioctl+0x1da/0x270 fs/ioctl.c:718
    __x64_sys_ioctl+0x4a/0x70 fs/ioctl.c:718
    do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
    entry_SYSCALL_64_after_hwframe+0x63/0xe7
    RIP: 0033:0x440019
    Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 3d 01 f0 ff ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
    RSP: 002b:00007ffdd4659fb8 EFLAGS: 00000213 ORIG_RAX: 0000000000000010
    RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440019
    RDX: 0000000020000080 RSI: 00000000c0044306 RDI: 0000000000000003
    RBP: 00000000006ca018 R08: 0000000000000000 R09: 00000000004002c8
    R10: 0000000000000000 R11: 0000000000000213 R12: 00000000004018a0
    R13: 0000000000401930 R14: 0000000000000000 R15: 0000000000000000

    Local variable description: ----data.i@capi_unlocked_ioctl
    Variable was created at:
    capi_ioctl drivers/isdn/capi/capi.c:747 [inline]
    capi_unlocked_ioctl+0x82/0x1bf0 drivers/isdn/capi/capi.c:939
    do_vfs_ioctl+0xebd/0x2bf0 fs/ioctl.c:46

    Bytes 12-63 of 64 are uninitialized
    Memory access of size 64 starts at ffff88807ac5fce8
    Data copied to user address 0000000020000080

    Signed-off-by: Eric Dumazet
    Reported-by: syzbot
    Cc: Karsten Keil
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Eric Dumazet
     

22 Aug, 2018

1 commit

  • [ Upstream commit 5e22002aa8809e2efab2da95855f73f63e14a36c ]

    It was possible to directly leak the kernel address where the isdn_dev
    structure pointer was stored. This is a kernel ASLR bypass for anyone
    with access to the ioctl. The code had been present since the beginning
    of git history, though this shouldn't ever be needed for normal operation,
    therefore remove it.

    Reported-by: Al Viro
    Cc: Karsten Keil
    Signed-off-by: Kees Cook
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Kees Cook
     

12 Jun, 2018

1 commit

  • [ Upstream commit 6009d1fe6ba3bb2dab55921da60465329cc1cd89 ]

    In divasmain.c, the function divas_write() firstly invokes the function
    diva_xdi_open_adapter() to open the adapter that matches with the adapter
    number provided by the user, and then invokes the function diva_xdi_write()
    to perform the write operation using the matched adapter. The two functions
    diva_xdi_open_adapter() and diva_xdi_write() are located in diva.c.

    In diva_xdi_open_adapter(), the user command is copied to the object 'msg'
    from the userspace pointer 'src' through the function pointer 'cp_fn',
    which eventually calls copy_from_user() to do the copy. Then, the adapter
    number 'msg.adapter' is used to find out a matched adapter from the
    'adapter_queue'. A matched adapter will be returned if it is found.
    Otherwise, NULL is returned to indicate the failure of the verification on
    the adapter number.

    As mentioned above, if a matched adapter is returned, the function
    diva_xdi_write() is invoked to perform the write operation. In this
    function, the user command is copied once again from the userspace pointer
    'src', which is the same as the 'src' pointer in diva_xdi_open_adapter() as
    both of them are from the 'buf' pointer in divas_write(). Similarly, the
    copy is achieved through the function pointer 'cp_fn', which finally calls
    copy_from_user(). After the successful copy, the corresponding command
    processing handler of the matched adapter is invoked to perform the write
    operation.

    It is obvious that there are two copies here from userspace, one is in
    diva_xdi_open_adapter(), and one is in diva_xdi_write(). Plus, both of
    these two copies share the same source userspace pointer, i.e., the 'buf'
    pointer in divas_write(). Given that a malicious userspace process can race
    to change the content pointed by the 'buf' pointer, this can pose potential
    security issues. For example, in the first copy, the user provides a valid
    adapter number to pass the verification process and a valid adapter can be
    found. Then the user can modify the adapter number to an invalid number.
    This way, the user can bypass the verification process of the adapter
    number and inject inconsistent data.

    This patch reuses the data copied in
    diva_xdi_open_adapter() and passes it to diva_xdi_write(). This way, the
    above issues can be avoided.

    Signed-off-by: Wenwen Wang
    Signed-off-by: David S. Miller
    Signed-off-by: Greg Kroah-Hartman

    Wenwen Wang
     

02 Nov, 2017

1 commit

  • Many source files in the tree are missing licensing information, which
    makes it harder for compliance tools to determine the correct license.

    By default all files without license information are under the default
    license of the kernel, which is GPL version 2.

    Update the files which contain no license information with the 'GPL-2.0'
    SPDX license identifier. The SPDX identifier is a legally binding
    shorthand, which can be used instead of the full boiler plate text.

    This patch is based on work done by Thomas Gleixner and Kate Stewart and
    Philippe Ombredanne.

    How this work was done:

    Patches were generated and checked against linux-4.14-rc6 for a subset of
    the use cases:
    - file had no licensing information it it.
    - file was a */uapi/* one with no licensing information in it,
    - file was a */uapi/* one with existing licensing information,

    Further patches will be generated in subsequent months to fix up cases
    where non-standard license headers were used, and references to license
    had to be inferred by heuristics based on keywords.

    The analysis to determine which SPDX License Identifier to be applied to
    a file was done in a spreadsheet of side by side results from of the
    output of two independent scanners (ScanCode & Windriver) producing SPDX
    tag:value files created by Philippe Ombredanne. Philippe prepared the
    base worksheet, and did an initial spot review of a few 1000 files.

    The 4.13 kernel was the starting point of the analysis with 60,537 files
    assessed. Kate Stewart did a file by file comparison of the scanner
    results in the spreadsheet to determine which SPDX license identifier(s)
    to be applied to the file. She confirmed any determination that was not
    immediately clear with lawyers working with the Linux Foundation.

    Criteria used to select files for SPDX license identifier tagging was:
    - Files considered eligible had to be source code files.
    - Make and config files were included as candidates if they contained >5
    lines of source
    - File already had some variant of a license header in it (even if
    Reviewed-by: Philippe Ombredanne
    Reviewed-by: Thomas Gleixner
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

21 Sep, 2017

1 commit

  • In isdn_ppp_write(), the header (i.e., protobuf) of the buffer is
    fetched twice from userspace. The first fetch is used to peek at the
    protocol of the message and reset the huptimer if necessary; while the
    second fetch copies in the whole buffer. However, given that buf resides
    in userspace memory, a user process can race to change its memory content
    across fetches. By doing so, we can either avoid resetting the huptimer
    for any type of packets (by first setting proto to PPP_LCP and later
    change to the actual type) or force resetting the huptimer for LCP
    packets.

    This patch changes this double-fetch behavior into two single fetches
    decided by condition (lp->isdn_device < 0 || lp->isdn_channel
    Signed-off-by: David S. Miller

    Meng Xu
     

08 Sep, 2017

1 commit

  • gcc-7 found an ancient bug in the loop driver, leading to a condition that
    is always false, meaning we ignore the contents of 'card->flags' here:

    drivers/isdn/isdnloop/isdnloop.c:412:37: error: ?: using integer constants in boolean context, the expression will always evaluate to 'true' [-Werror=int-in-bool-context]

    This changes the braces in the expression to ensure we actually
    compare the flag bits, rather than comparing a constant. As Joe Perches
    pointed out, an earlier patch of mine incorrectly assumed this was a
    false-positive warning.

    Cc: Joe Perches
    Link: https://patchwork.kernel.org/patch/9840289/
    Signed-off-by: Arnd Bergmann
    Signed-off-by: David S. Miller

    Arnd Bergmann
     

16 Aug, 2017

1 commit


12 Aug, 2017

1 commit

  • If mISDN_FsmNew() fails to allocate memory for jumpmatrix
    then null pointer dereference will occur on any write to
    jumpmatrix.

    The patch adds check on successful allocation and
    corresponding error handling.

    Found by Linux Driver Verification project (linuxtesting.org).

    Signed-off-by: Anton Vasilyev
    Signed-off-by: David S. Miller

    Anton Vasilyev
     

10 Aug, 2017

2 commits

  • The UDP offload conflict is dealt with by simply taking what is
    in net-next where we have removed all of the UFO handling code
    entirely.

    The TCP conflict was a case of local variables in a function
    being removed from both net and net-next.

    In netvsc we had an assignment right next to where a missing
    set of u64 stats sync object inits were added.

    Signed-off-by: David S. Miller

    David S. Miller
     
  • Pull networking fixes from David Miller:
    "The pull requests are getting smaller, that's progress I suppose :-)

    1) Fix infinite loop in CIPSO option parsing, from Yujuan Qi.

    2) Fix remote checksum handling in VXLAN and GUE tunneling drivers,
    from Koichiro Den.

    3) Missing u64_stats_init() calls in several drivers, from Florian
    Fainelli.

    4) TCP can set the congestion window to an invalid ssthresh value
    after congestion window reductions, from Yuchung Cheng.

    5) Fix BPF jit branch generation on s390, from Daniel Borkmann.

    6) Correct MIPS ebpf JIT merge, from David Daney.

    7) Correct byte order test in BPF test_verifier.c, from Daniel
    Borkmann.

    8) Fix various crashes and leaks in ASIX driver, from Dean Jenkins.

    9) Handle SCTP checksums properly in mlx4 driver, from Davide
    Caratti.

    10) We can potentially enter tcp_connect() with a cached route
    already, due to fastopen, so we have to explicitly invalidate it.

    11) skb_warn_bad_offload() can bark in legitimate situations, fix from
    Willem de Bruijn"

    * git://git.kernel.org/pub/scm/linux/kernel/git/davem/net: (52 commits)
    net: avoid skb_warn_bad_offload false positives on UFO
    qmi_wwan: fix NULL deref on disconnect
    ppp: fix xmit recursion detection on ppp channels
    rds: Reintroduce statistics counting
    tcp: fastopen: tcp_connect() must refresh the route
    net: sched: set xt_tgchk_param par.net properly in ipt_init_target
    net: dsa: mediatek: add adjust link support for user ports
    net/mlx4_en: don't set CHECKSUM_COMPLETE on SCTP packets
    qed: Fix a memory allocation failure test in 'qed_mcp_cmd_init()'
    hysdn: fix to a race condition in put_log_buffer
    s390/qeth: fix L3 next-hop in xmit qeth hdr
    asix: Fix small memory leak in ax88772_unbind()
    asix: Ensure asix_rx_fixup_info members are all reset
    asix: Add rx->ax_skb = NULL after usbnet_skb_return()
    bpf: fix selftest/bpf/test_pkt_md_access on s390x
    netvsc: fix race on sub channel creation
    bpf: fix byte order test in test_verifier
    xgene: Always get clk source, but ignore if it's missing for SGMII ports
    MIPS: Add missing file for eBPF JIT.
    bpf, s390: fix build for libbpf and selftest suite
    ...

    Linus Torvalds
     

09 Aug, 2017

2 commits


08 Aug, 2017

3 commits

  • Declare this structure as const as it is only used during a copy
    operation.

    Signed-off-by: Bhumika Goyal
    Signed-off-by: David S. Miller

    Bhumika Goyal
     
  • The synchronization type that was used earlier to guard the loop that
    deletes unused log buffers may lead to a situation that prevents any
    thread from going through the loop.

    The patch deletes previously used synchronization mechanism and moves
    the loop under the spin_lock so the similar cases won't be feasible in
    the future.

    Found by by Linux Driver Verification project (linuxtesting.org).

    Signed-off-by: Anton Volkov
    Signed-off-by: David S. Miller

    Anton Volkov
     
  • Saeed Mahameed says:

    ====================
    mlx5-shared-2017-08-07

    This series includes some mlx5 updates for both net-next and rdma trees.

    From Saeed,
    Core driver updates to allow selectively building the driver with
    or without some large driver components, such as
    - E-Switch (Ethernet SRIOV support).
    - Multi-Physical Function Switch (MPFs) support.
    For that we split E-Switch and MPFs functionalities into separate files.

    From Erez,
    Delay mlx5_core events when mlx5 interfaces, namely mlx5_ib, registration
    is taking place and until it completes.

    From Rabie,
    Increase the maximum supported flow counters.
    ====================

    Signed-off-by: David S. Miller

    David S. Miller
     

03 Aug, 2017

1 commit

  • This fixes a potential buffer overflow in isdn_net.c caused by an
    unbounded strcpy.

    [ ISDN seems to be effectively unmaintained, and the I4L driver in
    particular is long deprecated, but in case somebody uses this..
    - Linus ]

    Signed-off-by: Jiten Thakkar
    Signed-off-by: Annie Cherkaev
    Cc: Karsten Keil
    Cc: Kees Cook
    Cc: stable@kernel.org
    Signed-off-by: Linus Torvalds

    Annie Cherkaev
     

21 Jul, 2017

1 commit


20 Jul, 2017

1 commit

  • Two arrays are clearly bit maps, so, make that explicit by converting to
    bitmap API and remove custom helpers.

    Note sig_ind() uses out of boundary bit to (looks like) protect against
    potential bitmap_empty() checks for the same bitmap.

    This patch removes that since:
    1) that didn't guarantee atomicity anyway;
    2) the first operation inside the for-loop is set bit in the bitmap
    (which effectively makes it non-empty);
    3) group_optimization() doesn't utilize possible emptiness of the bitmap
    in question.

    Thus, if there is a protection needed it should be implemented properly.

    Signed-off-by: Andy Shevchenko
    Signed-off-by: David S. Miller

    Andy Shevchenko
     

16 Jul, 2017

10 commits

  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    11803 544 1 12348 303c isdn/hardware/avm/c4.o

    File size After adding 'const':
    text data bss dec hex filename
    11931 416 1 12348 303c isdn/hardware/avm/c4.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    21656 1024 96 22776 58f8 isdn/hardware/mISDN/hfcpci.o

    File size After adding 'const':
    text data bss dec hex filename
    22424 256 96 22776 58f8 isdn/hardware/mISDN/hfcpci.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    9963 1936 16 11915 2e8b isdn/hardware/mISDN/avmfritz.o

    File size After adding 'const':
    text data bss dec hex filename
    10091 1808 16 11915 2e8b isdn/hardware/mISDN/avmfritz.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    13959 4080 24 18063 468f isdn/hardware/mISDN/w6692.o

    File size After adding 'const':
    text data bss dec hex filename
    14087 3952 24 18063 468f isdn/hardware/mISDN/w6692.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    63450 1536 1492 66478 103ae isdn/hardware/mISDN/hfcmulti.o

    File size After adding 'const':
    text data bss dec hex filename
    64698 288 1492 66478 103ae isdn/hardware/mISDN/hfcmulti.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    10941 1776 16 12733 31bd isdn/hardware/mISDN/netjet.o

    File size After adding 'const':
    text data bss dec hex filename
    11005 1712 16 12733 31bd isdn/hardware/mISDN/netjet.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    6224 655 8 6887 1ae7 isdn/hardware/eicon/divasmain.o

    File size After adding 'const':
    text data bss dec hex filename
    6608 271 8 6887 1ae7 isdn/hardware/eicon/divasmain.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    5989 576 0 6565 19a5 isdn/hisax/hisax_fcpcipnp.o

    File size After adding 'const':
    text data bss dec hex filename
    6085 480 0 6565 19a5 isdn/hisax/hisax_fcpcipnp.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    10512 536 4 11052 2b2c drivers/isdn/hisax/hfc4s8s_l1.o

    File size After adding 'const':
    text data bss dec hex filename
    10672 376 4 11052 2b2c drivers/isdn/hisax/hfc4s8s_l1.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     
  • pci_device_id are not supposed to change at runtime. All functions
    working with pci_device_id provided by work with
    const pci_device_id. So mark the non-const structs as const.

    File size before:
    text data bss dec hex filename
    13686 2064 4416 20166 4ec6 drivers/isdn/hisax/config.o

    File size After adding 'const':
    text data bss dec hex filename
    15030 720 4416 20166 4ec6 drivers/isdn/hisax/config.o

    Signed-off-by: Arvind Yadav
    Signed-off-by: David S. Miller

    Arvind Yadav
     

15 Jul, 2017

1 commit

  • One string we pass into the cs->info buffer might be too long,
    as pointed out by gcc:

    drivers/isdn/divert/isdn_divert.c: In function 'll_callback':
    drivers/isdn/divert/isdn_divert.c:488:22: error: '%d' directive writing between 1 and 3 bytes into a region of size between 1 and 69 [-Werror=format-overflow=]
    sprintf(cs->info, "%d 0x%lx %s %s %s %s 0x%x 0x%x %d %d %s\n",
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    drivers/isdn/divert/isdn_divert.c:488:22: note: directive argument in the range [0, 255]
    drivers/isdn/divert/isdn_divert.c:488:4: note: 'sprintf' output 25 or more bytes (assuming 129) into a destination of size 90

    This is unlikely to actually cause problems, so let's use snprintf
    as a simple workaround to shut up the warning and truncate the
    buffer instead.

    Signed-off-by: Arnd Bergmann
    Signed-off-by: David S. Miller

    Arnd Bergmann
     

06 Jul, 2017

1 commit

  • Pull misc user access cleanups from Al Viro:
    "The first pile is assorted getting rid of cargo-culted access_ok(),
    cargo-culted set_fs() and field-by-field copyouts.

    The same description applies to a lot of stuff in other branches -
    this is just the stuff that didn't fit into a more specific topical
    branch"

    * 'work.misc-set_fs' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs:
    Switch flock copyin/copyout primitives to copy_{from,to}_user()
    fs/fcntl: return -ESRCH in f_setown when pid/pgid can't be found
    fs/fcntl: f_setown, avoid undefined behaviour
    fs/fcntl: f_setown, allow returning error
    lpfc debugfs: get rid of pointless access_ok()
    adb: get rid of pointless access_ok()
    isdn: get rid of pointless access_ok()
    compat statfs: switch to copy_to_user()
    fs/locks: don't mess with the address limit in compat_fcntl64
    nfsd_readlink(): switch to vfs_get_link()
    drbd: ->sendpage() never needed set_fs()
    fs/locks: pass kernel struct flock to fcntl_getlk/setlk
    fs: locks: Fix some troubles at kernel-doc comments

    Linus Torvalds
     

01 Jul, 2017

1 commit

  • refcount_t type and corresponding API should be
    used instead of atomic_t when the variable is used as
    a reference counter. This allows to avoid accidental
    refcounter overflows that might lead to use-after-free
    situations.

    Signed-off-by: Elena Reshetova
    Signed-off-by: Hans Liljestrand
    Signed-off-by: Kees Cook
    Signed-off-by: David Windsor
    Signed-off-by: David S. Miller

    Reshetova, Elena
     

21 Jun, 2017

3 commits

  • in my commit b952f4dff2751252db073c27c0f8a16a416a2ddc,
    - *(u8 *)skb_put(skb_out, 1) = (u8)(accm >> 24); \
    + skb_put(skb_out, (u8)(accm >> 24)); \
    it should skb_put_u8()

    Fixes: b952f4dff275 ("net: manual clean code which call skb_put_[data:zero])")
    Signed-off-by: yuan linyu
    Signed-off-by: David S. Miller

    yuan linyu
     
  • Signed-off-by: yuan linyu
    Signed-off-by: David S. Miller

    yuan linyu
     
  • follow Johannes Berg, semantic patch file as below,
    @@
    identifier p, p2;
    expression len;
    expression skb;
    type t, t2;
    @@
    (
    -p = __skb_put(skb, len);
    +p = __skb_put_zero(skb, len);
    |
    -p = (t)__skb_put(skb, len);
    +p = __skb_put_zero(skb, len);
    )
    ... when != p
    (
    p2 = (t2)p;
    -memset(p2, 0, len);
    |
    -memset(p, 0, len);
    )

    @@
    identifier p;
    expression len;
    expression skb;
    type t;
    @@
    (
    -t p = __skb_put(skb, len);
    +t p = __skb_put_zero(skb, len);
    )
    ... when != p
    (
    -memset(p, 0, len);
    )

    @@
    type t, t2;
    identifier p, p2;
    expression skb;
    @@
    t *p;
    ...
    (
    -p = __skb_put(skb, sizeof(t));
    +p = __skb_put_zero(skb, sizeof(t));
    |
    -p = (t *)__skb_put(skb, sizeof(t));
    +p = __skb_put_zero(skb, sizeof(t));
    )
    ... when != p
    (
    p2 = (t2)p;
    -memset(p2, 0, sizeof(*p));
    |
    -memset(p, 0, sizeof(*p));
    )

    @@
    expression skb, len;
    @@
    -memset(__skb_put(skb, len), 0, len);
    +__skb_put_zero(skb, len);

    @@
    expression skb, len, data;
    @@
    -memcpy(__skb_put(skb, len), data, len);
    +__skb_put_data(skb, data, len);

    @@
    expression SKB, C, S;
    typedef u8;
    identifier fn = {__skb_put};
    fresh identifier fn2 = fn ## "_u8";
    @@
    - *(u8 *)fn(SKB, S) = C;
    + fn2(SKB, C);

    Signed-off-by: yuan linyu
    Signed-off-by: David S. Miller

    yuan linyu
     

16 Jun, 2017

5 commits

  • Joe and Bjørn suggested that it'd be nicer to not have the
    cast in the fairly common case of doing
    *(u8 *)skb_put(skb, 1) = c;

    Add skb_put_u8() for this case, and use it across the code,
    using the following spatch:

    @@
    expression SKB, C, S;
    typedef u8;
    identifier fn = {skb_put};
    fresh identifier fn2 = fn ## "_u8";
    @@
    - *(u8 *)fn(SKB, S) = C;
    + fn2(SKB, C);

    Note that due to the "S", the spatch isn't perfect, it should
    have checked that S is 1, but there's also places that use a
    sizeof expression like sizeof(var) or sizeof(u8) etc. Turns
    out that nobody ever did something like
    *(u8 *)skb_put(skb, 2) = c;

    which would be wrong anyway since the second byte wouldn't be
    initialized.

    Suggested-by: Joe Perches
    Suggested-by: Bjørn Mork
    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions return void * and remove all the casts across
    the tree, adding a (u8 *) cast only where the unsigned char pointer
    was used directly, all done with the following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    @@
    expression SKB, LEN;
    identifier fn = { skb_push, __skb_push, skb_push_rcsum };
    @@
    - fn(SKB, LEN)[0]
    + *(u8 *)fn(SKB, LEN)

    Note that the last part there converts from push(...)[0] to the
    more idiomatic *(u8 *)push(...).

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions return void * and remove all the casts across
    the tree, adding a (u8 *) cast only where the unsigned char pointer
    was used directly, all done with the following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = {
    skb_pull,
    __skb_pull,
    skb_pull_inline,
    __pskb_pull_tail,
    __pskb_pull,
    pskb_pull
    };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = {
    skb_pull,
    __skb_pull,
    skb_pull_inline,
    __pskb_pull_tail,
    __pskb_pull,
    pskb_pull
    };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • It seems like a historic accident that these return unsigned char *,
    and in many places that means casts are required, more often than not.

    Make these functions (skb_put, __skb_put and pskb_put) return void *
    and remove all the casts across the tree, adding a (u8 *) cast only
    where the unsigned char pointer was used directly, all done with the
    following spatch:

    @@
    expression SKB, LEN;
    typedef u8;
    identifier fn = { skb_put, __skb_put };
    @@
    - *(fn(SKB, LEN))
    + *(u8 *)fn(SKB, LEN)

    @@
    expression E, SKB, LEN;
    identifier fn = { skb_put, __skb_put };
    type T;
    @@
    - E = ((T *)(fn(SKB, LEN)))
    + E = fn(SKB, LEN)

    which actually doesn't cover pskb_put since there are only three
    users overall.

    A handful of stragglers were converted manually, notably a macro in
    drivers/isdn/i4l/isdn_bsdcomp.c and, oddly enough, one of the many
    instances in net/bluetooth/hci_sock.c. In the former file, I also
    had to fix one whitespace problem spatch introduced.

    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     
  • A common pattern with skb_put() is to just want to memcpy()
    some data into the new space, introduce skb_put_data() for
    this.

    An spatch similar to the one for skb_put_zero() converts many
    of the places using it:

    @@
    identifier p, p2;
    expression len, skb, data;
    type t, t2;
    @@
    (
    -p = skb_put(skb, len);
    +p = skb_put_data(skb, data, len);
    |
    -p = (t)skb_put(skb, len);
    +p = skb_put_data(skb, data, len);
    )
    (
    p2 = (t2)p;
    -memcpy(p2, data, len);
    |
    -memcpy(p, data, len);
    )

    @@
    type t, t2;
    identifier p, p2;
    expression skb, data;
    @@
    t *p;
    ...
    (
    -p = skb_put(skb, sizeof(t));
    +p = skb_put_data(skb, data, sizeof(t));
    |
    -p = (t *)skb_put(skb, sizeof(t));
    +p = skb_put_data(skb, data, sizeof(t));
    )
    (
    p2 = (t2)p;
    -memcpy(p2, data, sizeof(*p));
    |
    -memcpy(p, data, sizeof(*p));
    )

    @@
    expression skb, len, data;
    @@
    -memcpy(skb_put(skb, len), data, len);
    +skb_put_data(skb, data, len);

    (again, manually post-processed to retain some comments)

    Reviewed-by: Stephen Hemminger
    Signed-off-by: Johannes Berg
    Signed-off-by: David S. Miller

    Johannes Berg
     

08 Jun, 2017

1 commit