| Message ID | 20210321163210.GC26497@amd (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net/dev: fix information leak to userspace | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: atenart@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 10 this patch: 10 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | CHECK: From:/Signed-off-by: email comments mismatch: 'From: Pavel Machek <pavel@denx.de>' != 'Signed-off-by: Pavel Machek (CIP) <pavel@denx.de>' |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 10 this patch: 10 |
| netdev/header_inline | success | Link |
On Sun, Mar 21, 2021 at 9:34 AM Pavel Machek <pavel@denx.de> wrote: > > dev_get_mac_address() does not always initialize whole > structure. Unfortunately, other code copies such structure to > userspace, leaking information. Fix it. Well, most callers already initialize it with a memset() or copy_from_user(), for example, __tun_chr_ioctl(): if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || (_IOC_TYPE(cmd) == SOCK_IOC_TYPE && cmd != SIOCGSKNS)) { if (copy_from_user(&ifr, argp, ifreq_len)) return -EFAULT; } else { memset(&ifr, 0, sizeof(ifr)); } Except tap_ioctl(), but we can just initialize 'sa' there instead of doing it in dev_get_mac_address(). Thanks.
diff --git a/net/core/dev.c b/net/core/dev.c index 6c5967e80132..28283a9eb63a 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -8949,11 +8949,9 @@ int dev_get_mac_address(struct sockaddr *sa, struct net *net, char *dev_name) ret = -ENODEV; goto unlock; } - if (!dev->addr_len) - memset(sa->sa_data, 0, size); - else - memcpy(sa->sa_data, dev->dev_addr, - min_t(size_t, size, dev->addr_len)); + memset(sa->sa_data, 0, size); + memcpy(sa->sa_data, dev->dev_addr, + min_t(size_t, size, dev->addr_len)); sa->sa_family = dev->type; unlock:
dev_get_mac_address() does not always initialize whole structure. Unfortunately, other code copies such structure to userspace, leaking information. Fix it. Signed-off-by: Pavel Machek (CIP) <pavel@denx.de> Cc: stable@kernel.org