Skip to content

Commit

Permalink
Merge tag 'hardening-v6.8-rc1' of git://git.kernel.org/pub/scm/linux/…
Browse files Browse the repository at this point in the history
…kernel/git/kees/linux

Pull hardening updates from Kees Cook:

 - Introduce the param_unknown_fn type and other clean ups (Andy
   Shevchenko)

 - Various __counted_by annotations (Christophe JAILLET, Gustavo A. R.
   Silva, Kees Cook)

 - Add KFENCE test to LKDTM (Stephen Boyd)

 - Various strncpy() refactorings (Justin Stitt)

 - Fix qnx4 to avoid writing into the smaller of two overlapping buffers

 - Various strlcpy() refactorings

* tag 'hardening-v6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux:
  qnx4: Use get_directory_fname() in qnx4_match()
  qnx4: Extract dir entry filename processing into helper
  atags_proc: Add __counted_by for struct buffer and use struct_size()
  tracing/uprobe: Replace strlcpy() with strscpy()
  params: Fix multi-line comment style
  params: Sort headers
  params: Use size_add() for kmalloc()
  params: Do not go over the limit when getting the string length
  params: Introduce the param_unknown_fn type
  lkdtm: Add kfence read after free crash type
  nvme-fc: replace deprecated strncpy with strscpy
  nvdimm/btt: replace deprecated strncpy with strscpy
  nvme-fabrics: replace deprecated strncpy with strscpy
  drm/modes: replace deprecated strncpy with strscpy_pad
  afs: Add __counted_by for struct afs_acl and use struct_size()
  VMCI: Annotate struct vmci_handle_arr with __counted_by
  i40e: Annotate struct i40e_qvlist_info with __counted_by
  HID: uhid: replace deprecated strncpy with strscpy
  samples: Replace strlcpy() with strscpy()
  SUNRPC: Replace strlcpy() with strscpy()
  • Loading branch information
Linus Torvalds committed Jan 10, 2024
2 parents 72116ef + a75b380 commit 120a201
Show file tree
Hide file tree
Showing 21 changed files with 208 additions and 124 deletions.
4 changes: 2 additions & 2 deletions arch/arm/kernel/atags_proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

struct buffer {
size_t size;
char data[];
char data[] __counted_by(size);
};

static ssize_t atags_read(struct file *file, char __user *buf,
Expand Down Expand Up @@ -54,7 +54,7 @@ static int __init init_atags_procfs(void)

WARN_ON(tag->hdr.tag != ATAG_NONE);

b = kmalloc(sizeof(*b) + size, GFP_KERNEL);
b = kmalloc(struct_size(b, data, size), GFP_KERNEL);
if (!b)
goto nomem;

Expand Down
6 changes: 2 additions & 4 deletions drivers/gpu/drm/drm_modes.c
Original file line number Diff line number Diff line change
Expand Up @@ -2617,8 +2617,7 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out,
break;
}

strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
strscpy_pad(out->name, in->name, sizeof(out->name));
}

/**
Expand Down Expand Up @@ -2659,8 +2658,7 @@ int drm_mode_convert_umode(struct drm_device *dev,
* useful for the kernel->userspace direction anyway.
*/
out->type = in->type & DRM_MODE_TYPE_ALL;
strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
strscpy_pad(out->name, in->name, sizeof(out->name));

/* Clearing picture aspect ratio bits from out flags,
* as the aspect-ratio information is not stored in
Expand Down
15 changes: 7 additions & 8 deletions drivers/hid/uhid.c
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
const struct uhid_event *ev)
{
struct hid_device *hid;
size_t rd_size, len;
size_t rd_size;
void *rd_data;
int ret;

Expand All @@ -514,13 +514,12 @@ static int uhid_dev_create2(struct uhid_device *uhid,
goto err_free;
}

/* @hid is zero-initialized, strncpy() is correct, strlcpy() not */
len = min(sizeof(hid->name), sizeof(ev->u.create2.name)) - 1;
strncpy(hid->name, ev->u.create2.name, len);
len = min(sizeof(hid->phys), sizeof(ev->u.create2.phys)) - 1;
strncpy(hid->phys, ev->u.create2.phys, len);
len = min(sizeof(hid->uniq), sizeof(ev->u.create2.uniq)) - 1;
strncpy(hid->uniq, ev->u.create2.uniq, len);
BUILD_BUG_ON(sizeof(hid->name) != sizeof(ev->u.create2.name));
strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));

hid->ll_driver = &uhid_hid_driver;
hid->bus = ev->u.create2.bus;
Expand Down
60 changes: 60 additions & 0 deletions drivers/misc/lkdtm/heap.c
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* page allocation and slab allocations.
*/
#include "lkdtm.h"
#include <linux/kfence.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
#include <linux/sched.h>
Expand Down Expand Up @@ -132,6 +133,64 @@ static void lkdtm_READ_AFTER_FREE(void)
kfree(val);
}

static void lkdtm_KFENCE_READ_AFTER_FREE(void)
{
int *base, val, saw;
unsigned long timeout, resched_after;
size_t len = 1024;
/*
* The slub allocator will use the either the first word or
* the middle of the allocation to store the free pointer,
* depending on configurations. Store in the second word to
* avoid running into the freelist.
*/
size_t offset = sizeof(*base);

/*
* 100x the sample interval should be more than enough to ensure we get
* a KFENCE allocation eventually.
*/
timeout = jiffies + msecs_to_jiffies(100 * kfence_sample_interval);
/*
* Especially for non-preemption kernels, ensure the allocation-gate
* timer can catch up: after @resched_after, every failed allocation
* attempt yields, to ensure the allocation-gate timer is scheduled.
*/
resched_after = jiffies + msecs_to_jiffies(kfence_sample_interval);
do {
base = kmalloc(len, GFP_KERNEL);
if (!base) {
pr_err("FAIL: Unable to allocate kfence memory!\n");
return;
}

if (is_kfence_address(base)) {
val = 0x12345678;
base[offset] = val;
pr_info("Value in memory before free: %x\n", base[offset]);

kfree(base);

pr_info("Attempting bad read from freed memory\n");
saw = base[offset];
if (saw != val) {
/* Good! Poisoning happened, so declare a win. */
pr_info("Memory correctly poisoned (%x)\n", saw);
} else {
pr_err("FAIL: Memory was not poisoned!\n");
pr_expected_config_param(CONFIG_INIT_ON_FREE_DEFAULT_ON, "init_on_free");
}
return;
}

kfree(base);
if (time_after(jiffies, resched_after))
cond_resched();
} while (time_before(jiffies, timeout));

pr_err("FAIL: kfence memory never allocated!\n");
}

static void lkdtm_WRITE_BUDDY_AFTER_FREE(void)
{
unsigned long p = __get_free_page(GFP_KERNEL);
Expand Down Expand Up @@ -327,6 +386,7 @@ static struct crashtype crashtypes[] = {
CRASHTYPE(VMALLOC_LINEAR_OVERFLOW),
CRASHTYPE(WRITE_AFTER_FREE),
CRASHTYPE(READ_AFTER_FREE),
CRASHTYPE(KFENCE_READ_AFTER_FREE),
CRASHTYPE(WRITE_BUDDY_AFTER_FREE),
CRASHTYPE(READ_BUDDY_AFTER_FREE),
CRASHTYPE(SLAB_INIT_ON_ALLOC),
Expand Down
2 changes: 1 addition & 1 deletion drivers/misc/vmw_vmci/vmci_handle_array.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ struct vmci_handle_arr {
u32 max_capacity;
u32 size;
u32 pad;
struct vmci_handle entries[];
struct vmci_handle entries[] __counted_by(capacity);
};

#define VMCI_HANDLE_ARRAY_HEADER_SIZE \
Expand Down
2 changes: 1 addition & 1 deletion drivers/nvdimm/btt.c
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ static int btt_arena_write_layout(struct arena_info *arena)
if (!super)
return -ENOMEM;

strncpy(super->signature, BTT_SIG, BTT_SIG_LEN);
strscpy(super->signature, BTT_SIG, sizeof(super->signature));
export_uuid(super->uuid, nd_btt->uuid);
export_uuid(super->parent_uuid, parent_uuid);
super->flags = cpu_to_le32(arena->flags);
Expand Down
4 changes: 2 additions & 2 deletions drivers/nvme/host/fabrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ static struct nvmf_connect_data *nvmf_connect_data_prep(struct nvme_ctrl *ctrl,

uuid_copy(&data->hostid, &ctrl->opts->host->id);
data->cntlid = cpu_to_le16(cntlid);
strncpy(data->subsysnqn, ctrl->opts->subsysnqn, NVMF_NQN_SIZE);
strncpy(data->hostnqn, ctrl->opts->host->nqn, NVMF_NQN_SIZE);
strscpy(data->subsysnqn, ctrl->opts->subsysnqn, NVMF_NQN_SIZE);
strscpy(data->hostnqn, ctrl->opts->host->nqn, NVMF_NQN_SIZE);

return data;
}
Expand Down
8 changes: 4 additions & 4 deletions drivers/nvme/host/fc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1218,10 +1218,10 @@ nvme_fc_connect_admin_queue(struct nvme_fc_ctrl *ctrl,
/* Linux supports only Dynamic controllers */
assoc_rqst->assoc_cmd.cntlid = cpu_to_be16(0xffff);
uuid_copy(&assoc_rqst->assoc_cmd.hostid, &ctrl->ctrl.opts->host->id);
strncpy(assoc_rqst->assoc_cmd.hostnqn, ctrl->ctrl.opts->host->nqn,
min(FCNVME_ASSOC_HOSTNQN_LEN, NVMF_NQN_SIZE));
strncpy(assoc_rqst->assoc_cmd.subnqn, ctrl->ctrl.opts->subsysnqn,
min(FCNVME_ASSOC_SUBNQN_LEN, NVMF_NQN_SIZE));
strscpy(assoc_rqst->assoc_cmd.hostnqn, ctrl->ctrl.opts->host->nqn,
sizeof(assoc_rqst->assoc_cmd.hostnqn));
strscpy(assoc_rqst->assoc_cmd.subnqn, ctrl->ctrl.opts->subsysnqn,
sizeof(assoc_rqst->assoc_cmd.subnqn));

lsop->queue = queue;
lsreq->rqstaddr = assoc_rqst;
Expand Down
2 changes: 1 addition & 1 deletion fs/afs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ extern void afs_fs_inline_bulk_status(struct afs_operation *);

struct afs_acl {
u32 size;
u8 data[];
u8 data[] __counted_by(size);
};

extern void afs_fs_fetch_acl(struct afs_operation *);
Expand Down
2 changes: 1 addition & 1 deletion fs/afs/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static bool afs_make_acl(struct afs_operation *op,
{
struct afs_acl *acl;

acl = kmalloc(sizeof(*acl) + size, GFP_KERNEL);
acl = kmalloc(struct_size(acl, data, size), GFP_KERNEL);
if (!acl) {
afs_op_nomem(op);
return false;
Expand Down
52 changes: 7 additions & 45 deletions fs/qnx4/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,43 +15,6 @@
#include <linux/buffer_head.h>
#include "qnx4.h"

/*
* A qnx4 directory entry is an inode entry or link info
* depending on the status field in the last byte. The
* first byte is where the name start either way, and a
* zero means it's empty.
*
* Also, due to a bug in gcc, we don't want to use the
* real (differently sized) name arrays in the inode and
* link entries, but always the 'de_name[]' one in the
* fake struct entry.
*
* See
*
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c6
*
* for details, but basically gcc will take the size of the
* 'name' array from one of the used union entries randomly.
*
* This use of 'de_name[]' (48 bytes) avoids the false positive
* warnings that would happen if gcc decides to use 'inode.di_name'
* (16 bytes) even when the pointer and size were to come from
* 'link.dl_name' (48 bytes).
*
* In all cases the actual name pointer itself is the same, it's
* only the gcc internal 'what is the size of this field' logic
* that can get confused.
*/
union qnx4_directory_entry {
struct {
const char de_name[48];
u8 de_pad[15];
u8 de_status;
};
struct qnx4_inode_entry inode;
struct qnx4_link_info link;
};

static int qnx4_readdir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
Expand All @@ -74,26 +37,25 @@ static int qnx4_readdir(struct file *file, struct dir_context *ctx)
ix = (ctx->pos >> QNX4_DIR_ENTRY_SIZE_BITS) % QNX4_INODES_PER_BLOCK;
for (; ix < QNX4_INODES_PER_BLOCK; ix++, ctx->pos += QNX4_DIR_ENTRY_SIZE) {
union qnx4_directory_entry *de;
const char *fname;

offset = ix * QNX4_DIR_ENTRY_SIZE;
de = (union qnx4_directory_entry *) (bh->b_data + offset);

if (!de->de_name[0])
continue;
if (!(de->de_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
fname = get_entry_fname(de, &size);
if (!fname)
continue;

if (!(de->de_status & QNX4_FILE_LINK)) {
size = sizeof(de->inode.di_fname);
ino = blknum * QNX4_INODES_PER_BLOCK + ix - 1;
} else {
size = sizeof(de->link.dl_fname);
ino = ( le32_to_cpu(de->link.dl_inode_blk) - 1 ) *
QNX4_INODES_PER_BLOCK +
de->link.dl_inode_ndx;
}
size = strnlen(de->de_name, size);
QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, name));
if (!dir_emit(ctx, de->de_name, size, ino, DT_UNKNOWN)) {

QNX4DEBUG((KERN_INFO "qnx4_readdir:%.*s\n", size, fname));
if (!dir_emit(ctx, fname, size, ino, DT_UNKNOWN)) {
brelse(bh);
return 0;
}
Expand Down
29 changes: 11 additions & 18 deletions fs/qnx4/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,24 @@
static int qnx4_match(int len, const char *name,
struct buffer_head *bh, unsigned long *offset)
{
struct qnx4_inode_entry *de;
int namelen, thislen;
union qnx4_directory_entry *de;
const char *fname;
int fnamelen;

if (bh == NULL) {
printk(KERN_WARNING "qnx4: matching unassigned buffer !\n");
return 0;
}
de = (struct qnx4_inode_entry *) (bh->b_data + *offset);
de = (union qnx4_directory_entry *) (bh->b_data + *offset);
*offset += QNX4_DIR_ENTRY_SIZE;
if ((de->di_status & QNX4_FILE_LINK) != 0) {
namelen = QNX4_NAME_MAX;
} else {
namelen = QNX4_SHORT_NAME_MAX;
}
thislen = strlen( de->di_fname );
if ( thislen > namelen )
thislen = namelen;
if (len != thislen) {

fname = get_entry_fname(de, &fnamelen);
if (!fname || len != fnamelen)
return 0;
}
if (strncmp(name, de->di_fname, len) == 0) {
if ((de->di_status & (QNX4_FILE_USED|QNX4_FILE_LINK)) != 0) {
return 1;
}
}

if (strncmp(name, fname, len) == 0)
return 1;

return 0;
}

Expand Down
60 changes: 60 additions & 0 deletions fs/qnx4/qnx4.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,63 @@ static inline struct qnx4_inode_entry *qnx4_raw_inode(struct inode *inode)
{
return &qnx4_i(inode)->raw;
}

/*
* A qnx4 directory entry is an inode entry or link info
* depending on the status field in the last byte. The
* first byte is where the name start either way, and a
* zero means it's empty.
*
* Also, due to a bug in gcc, we don't want to use the
* real (differently sized) name arrays in the inode and
* link entries, but always the 'de_name[]' one in the
* fake struct entry.
*
* See
*
* https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578#c6
*
* for details, but basically gcc will take the size of the
* 'name' array from one of the used union entries randomly.
*
* This use of 'de_name[]' (48 bytes) avoids the false positive
* warnings that would happen if gcc decides to use 'inode.di_name'
* (16 bytes) even when the pointer and size were to come from
* 'link.dl_name' (48 bytes).
*
* In all cases the actual name pointer itself is the same, it's
* only the gcc internal 'what is the size of this field' logic
* that can get confused.
*/
union qnx4_directory_entry {
struct {
const char de_name[48];
u8 de_pad[15];
u8 de_status;
};
struct qnx4_inode_entry inode;
struct qnx4_link_info link;
};

static inline const char *get_entry_fname(union qnx4_directory_entry *de,
int *size)
{
/* Make sure the status byte is in the same place for all structs. */
BUILD_BUG_ON(offsetof(struct qnx4_inode_entry, di_status) !=
offsetof(struct qnx4_link_info, dl_status));
BUILD_BUG_ON(offsetof(struct qnx4_inode_entry, di_status) !=
offsetof(union qnx4_directory_entry, de_status));

if (!de->de_name[0])
return NULL;
if (!(de->de_status & (QNX4_FILE_USED|QNX4_FILE_LINK)))
return NULL;
if (!(de->de_status & QNX4_FILE_LINK))
*size = sizeof(de->inode.di_fname);
else
*size = sizeof(de->link.dl_fname);

*size = strnlen(de->de_name, *size);

return de->de_name;
}
Loading

0 comments on commit 120a201

Please sign in to comment.