* [PATCH 01/17] KVM: x86: Replace memset() "optimization" with normal per-field writes
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
` (17 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, Sean Christopherson, linux-kernel
From: Sean Christopherson <seanjc@google.com>
Explicitly zero select fields in the emulator's decode cache instead of
zeroing the fields via a gross memset() that spans six fields. gcc and
clang are both clever enough to batch the first five fields into a single
quadword MOV, i.e. memset() and individually zeroing generate identical
code.
Removing the wart also prepares KVM for FORTIFY_SOURCE performing
compile-time and run-time field bounds checking for memset().
No functional change intended.
Reported-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/lkml/YR0jIEzEcUom/7rd@google.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/kvm/emulate.c | 9 +++++++--
arch/x86/kvm/kvm_emulate.h | 6 +-----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 28b1a4e57827..7401a3133e17 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -5381,8 +5381,13 @@ static int fastop(struct x86_emulate_ctxt *ctxt, fastop_t fop)
void init_decode_cache(struct x86_emulate_ctxt *ctxt)
{
- memset(&ctxt->rip_relative, 0,
- (void *)&ctxt->modrm - (void *)&ctxt->rip_relative);
+ /* Clear fields that are set conditionally but read without a guard. */
+ ctxt->rip_relative = false;
+ ctxt->rex_prefix = 0;
+ ctxt->lock_prefix = 0;
+ ctxt->rep_prefix = 0;
+ ctxt->regs_valid = 0;
+ ctxt->regs_dirty = 0;
ctxt->io_read.pos = 0;
ctxt->io_read.end = 0;
diff --git a/arch/x86/kvm/kvm_emulate.h b/arch/x86/kvm/kvm_emulate.h
index 68b420289d7e..bc1fecacccd4 100644
--- a/arch/x86/kvm/kvm_emulate.h
+++ b/arch/x86/kvm/kvm_emulate.h
@@ -336,11 +336,7 @@ struct x86_emulate_ctxt {
fastop_t fop;
};
int (*check_perm)(struct x86_emulate_ctxt *ctxt);
- /*
- * The following six fields are cleared together,
- * the rest are initialized unconditionally in x86_decode_insn
- * or elsewhere
- */
+
bool rip_relative;
u8 rex_prefix;
u8 lock_prefix;
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy()
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
2021-12-13 22:33 ` [PATCH 01/17] KVM: x86: Replace memset() "optimization" with normal per-field writes Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 03/17] net/mlx5e: Use struct_group() for memcpy() region Kees Cook
` (16 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Saeed Mahameed, Leon Romanovsky, David S. Miller,
Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
Jesper Dangaard Brouer, John Fastabend, netdev, linux-rdma, bpf,
linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
Use flexible arrays instead of zero-element arrays (which look like they
are always overflowing) and split the cross-field memcpy() into two halves
that can be appropriately bounds-checked by the compiler.
We were doing:
#define ETH_HLEN 14
#define VLAN_HLEN 4
...
#define MLX5E_XDP_MIN_INLINE (ETH_HLEN + VLAN_HLEN)
...
struct mlx5e_tx_wqe *wqe = mlx5_wq_cyc_get_wqe(wq, pi);
...
struct mlx5_wqe_eth_seg *eseg = &wqe->eth;
struct mlx5_wqe_data_seg *dseg = wqe->data;
...
memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
target is wqe->eth.inline_hdr.start (which the compiler sees as being
2 bytes in size), but copying 18, intending to write across start
(really vlan_tci, 2 bytes). The remaining 16 bytes get written into
wqe->data[0], covering byte_count (4 bytes), lkey (4 bytes), and addr
(8 bytes).
struct mlx5e_tx_wqe {
struct mlx5_wqe_ctrl_seg ctrl; /* 0 16 */
struct mlx5_wqe_eth_seg eth; /* 16 16 */
struct mlx5_wqe_data_seg data[]; /* 32 0 */
/* size: 32, cachelines: 1, members: 3 */
/* last cacheline: 32 bytes */
};
struct mlx5_wqe_eth_seg {
u8 swp_outer_l4_offset; /* 0 1 */
u8 swp_outer_l3_offset; /* 1 1 */
u8 swp_inner_l4_offset; /* 2 1 */
u8 swp_inner_l3_offset; /* 3 1 */
u8 cs_flags; /* 4 1 */
u8 swp_flags; /* 5 1 */
__be16 mss; /* 6 2 */
__be32 flow_table_metadata; /* 8 4 */
union {
struct {
__be16 sz; /* 12 2 */
u8 start[2]; /* 14 2 */
} inline_hdr; /* 12 4 */
struct {
__be16 type; /* 12 2 */
__be16 vlan_tci; /* 14 2 */
} insert; /* 12 4 */
__be32 trailer; /* 12 4 */
}; /* 12 4 */
/* size: 16, cachelines: 1, members: 9 */
/* last cacheline: 16 bytes */
};
struct mlx5_wqe_data_seg {
__be32 byte_count; /* 0 4 */
__be32 lkey; /* 4 4 */
__be64 addr; /* 8 8 */
/* size: 16, cachelines: 1, members: 3 */
/* last cacheline: 16 bytes */
};
So, split the memcpy() so the compiler can reason about the buffer
sizes.
"pahole" shows no size nor member offset changes to struct mlx5e_tx_wqe
nor struct mlx5e_umr_wqe. "objdump -d" shows no meaningful object
code changes (i.e. only source line number induced differences and
optimizations).
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jesper Dangaard Brouer <hawk@kernel.org>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Cc: bpf@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 6 +++---
drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 4 +++-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index e77c4159713f..5d8e0a712313 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -225,7 +225,7 @@ static inline int mlx5e_get_max_num_channels(struct mlx5_core_dev *mdev)
struct mlx5e_tx_wqe {
struct mlx5_wqe_ctrl_seg ctrl;
struct mlx5_wqe_eth_seg eth;
- struct mlx5_wqe_data_seg data[0];
+ struct mlx5_wqe_data_seg data[];
};
struct mlx5e_rx_wqe_ll {
@@ -242,8 +242,8 @@ struct mlx5e_umr_wqe {
struct mlx5_wqe_umr_ctrl_seg uctrl;
struct mlx5_mkey_seg mkc;
union {
- struct mlx5_mtt inline_mtts[0];
- struct mlx5_klm inline_klms[0];
+ DECLARE_FLEX_ARRAY(struct mlx5_mtt, inline_mtts);
+ DECLARE_FLEX_ARRAY(struct mlx5_klm, inline_klms);
};
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
index 2f0df5cc1a2d..efae2444c26f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
@@ -341,8 +341,10 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
/* copy the inline part if required */
if (sq->min_inline_mode != MLX5_INLINE_MODE_NONE) {
- memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
+ memcpy(eseg->inline_hdr.start, xdptxd->data, sizeof(eseg->inline_hdr.start));
eseg->inline_hdr.sz = cpu_to_be16(MLX5E_XDP_MIN_INLINE);
+ memcpy(dseg, xdptxd->data + sizeof(eseg->inline_hdr.start),
+ MLX5E_XDP_MIN_INLINE - sizeof(eseg->inline_hdr.start));
dma_len -= MLX5E_XDP_MIN_INLINE;
dma_addr += MLX5E_XDP_MIN_INLINE;
dseg++;
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 03/17] net/mlx5e: Use struct_group() for memcpy() region
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
2021-12-13 22:33 ` [PATCH 01/17] KVM: x86: Replace memset() "optimization" with normal per-field writes Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 04/17] media: omap3isp: " Kees Cook
` (15 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Saeed Mahameed, Leon Romanovsky, David S. Miller,
Jakub Kicinski, netdev, linux-rdma, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
Use struct_group() in struct vlan_ethhdr around members h_dest and
h_source, so they can be referenced together. This will allow memcpy()
and sizeof() to more easily reason about sizes, improve readability,
and avoid future warnings about writing beyond the end of h_dest.
"pahole" shows no size nor member offset changes to struct vlan_ethhdr.
"objdump -d" shows no object code changes.
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tx.c | 2 +-
include/linux/if_vlan.h | 6 ++++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index 7fd33b356cc8..ee7ecb88adc1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -208,7 +208,7 @@ static inline void mlx5e_insert_vlan(void *start, struct sk_buff *skb, u16 ihs)
int cpy1_sz = 2 * ETH_ALEN;
int cpy2_sz = ihs - cpy1_sz;
- memcpy(vhdr, skb->data, cpy1_sz);
+ memcpy(&vhdr->addrs, skb->data, cpy1_sz);
vhdr->h_vlan_proto = skb->vlan_proto;
vhdr->h_vlan_TCI = cpu_to_be16(skb_vlan_tag_get(skb));
memcpy(&vhdr->h_vlan_encapsulated_proto, skb->data + cpy1_sz, cpy2_sz);
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 8420fe504927..2be4dd7e90a9 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -46,8 +46,10 @@ struct vlan_hdr {
* @h_vlan_encapsulated_proto: packet type ID or len
*/
struct vlan_ethhdr {
- unsigned char h_dest[ETH_ALEN];
- unsigned char h_source[ETH_ALEN];
+ struct_group(addrs,
+ unsigned char h_dest[ETH_ALEN];
+ unsigned char h_source[ETH_ALEN];
+ );
__be16 h_vlan_proto;
__be16 h_vlan_TCI;
__be16 h_vlan_encapsulated_proto;
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 04/17] media: omap3isp: Use struct_group() for memcpy() region
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (2 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 03/17] net/mlx5e: Use struct_group() for memcpy() region Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 05/17] sata_fsl: " Kees Cook
` (14 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields. Wrap the target region
in struct_group(). This additionally fixes a theoretical misalignment
of the copy (since the size of "buf" changes between 64-bit and 32-bit,
but this is likely never built for 64-bit).
FWIW, I think this code is totally broken on 64-bit (which appears to
not be a "real" build configuration): it would either always fail (with
an uninitialized data->buf_size) or would cause corruption in userspace
due to the copy_to_user() in the call path against an uninitialized
data->buf value:
omap3isp_stat_request_statistics_time32(...)
struct omap3isp_stat_data data64;
...
omap3isp_stat_request_statistics(stat, &data64);
int omap3isp_stat_request_statistics(struct ispstat *stat,
struct omap3isp_stat_data *data)
...
buf = isp_stat_buf_get(stat, data);
static struct ispstat_buffer *isp_stat_buf_get(struct ispstat *stat,
struct omap3isp_stat_data *data)
...
if (buf->buf_size > data->buf_size) {
...
return ERR_PTR(-EINVAL);
}
...
rval = copy_to_user(data->buf,
buf->virt_addr,
buf->buf_size);
Regardless, additionally initialize data64 to be zero-filled to avoid
undefined behavior.
Fixes: 378e3f81cb56 ("media: omap3isp: support 64-bit version of omap3isp_stat_data")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/media/platform/omap3isp/ispstat.c | 5 +++--
include/uapi/linux/omap3isp.h | 21 +++++++++++++--------
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/media/platform/omap3isp/ispstat.c b/drivers/media/platform/omap3isp/ispstat.c
index 5b9b57f4d9bf..68cf68dbcace 100644
--- a/drivers/media/platform/omap3isp/ispstat.c
+++ b/drivers/media/platform/omap3isp/ispstat.c
@@ -512,7 +512,7 @@ int omap3isp_stat_request_statistics(struct ispstat *stat,
int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
struct omap3isp_stat_data_time32 *data)
{
- struct omap3isp_stat_data data64;
+ struct omap3isp_stat_data data64 = { };
int ret;
ret = omap3isp_stat_request_statistics(stat, &data64);
@@ -521,7 +521,8 @@ int omap3isp_stat_request_statistics_time32(struct ispstat *stat,
data->ts.tv_sec = data64.ts.tv_sec;
data->ts.tv_usec = data64.ts.tv_usec;
- memcpy(&data->buf, &data64.buf, sizeof(*data) - sizeof(data->ts));
+ data->buf = (uintptr_t)data64.buf;
+ memcpy(&data->frame, &data64.frame, sizeof(data->frame));
return 0;
}
diff --git a/include/uapi/linux/omap3isp.h b/include/uapi/linux/omap3isp.h
index 87b55755f4ff..d9db7ad43890 100644
--- a/include/uapi/linux/omap3isp.h
+++ b/include/uapi/linux/omap3isp.h
@@ -162,6 +162,7 @@ struct omap3isp_h3a_aewb_config {
* struct omap3isp_stat_data - Statistic data sent to or received from user
* @ts: Timestamp of returned framestats.
* @buf: Pointer to pass to user.
+ * @buf_size: Size of buffer.
* @frame_number: Frame number of requested stats.
* @cur_frame: Current frame number being processed.
* @config_counter: Number of the configuration associated with the data.
@@ -176,10 +177,12 @@ struct omap3isp_stat_data {
struct timeval ts;
#endif
void __user *buf;
- __u32 buf_size;
- __u16 frame_number;
- __u16 cur_frame;
- __u16 config_counter;
+ __struct_group(/* no tag */, frame, /* no attrs */,
+ __u32 buf_size;
+ __u16 frame_number;
+ __u16 cur_frame;
+ __u16 config_counter;
+ );
};
#ifdef __KERNEL__
@@ -189,10 +192,12 @@ struct omap3isp_stat_data_time32 {
__s32 tv_usec;
} ts;
__u32 buf;
- __u32 buf_size;
- __u16 frame_number;
- __u16 cur_frame;
- __u16 config_counter;
+ __struct_group(/* no tag */, frame, /* no attrs */,
+ __u32 buf_size;
+ __u16 frame_number;
+ __u16 cur_frame;
+ __u16 config_counter;
+ );
};
#endif
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 05/17] sata_fsl: Use struct_group() for memcpy() region
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (3 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 04/17] media: omap3isp: " Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
` (13 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, Jens Axboe, linux-ide, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memcpy(), memmove(), and memset(), avoid
intentionally writing across neighboring fields.
Use struct_group() in struct command_desc around members acmd and fill,
so they can be referenced together. This will allow memset(), memcpy(),
and sizeof() to more easily reason about sizes, improve readability,
and avoid future warnings about writing beyond the end of acmd:
In function 'fortify_memset_chk',
inlined from 'sata_fsl_qc_prep' at drivers/ata/sata_fsl.c:534:3:
./include/linux/fortify-string.h:199:4: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
199 | __write_overflow_field();
| ^~~~~~~~~~~~~~~~~~~~~~~~
Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-ide@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/ata/sata_fsl.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 3b31a4f596d8..c5a2c1e9ed6b 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -246,8 +246,10 @@ enum {
struct command_desc {
u8 cfis[8 * 4];
u8 sfis[8 * 4];
- u8 acmd[4 * 4];
- u8 fill[4 * 4];
+ struct_group(cdb,
+ u8 acmd[4 * 4];
+ u8 fill[4 * 4];
+ );
u32 prdt[SATA_FSL_MAX_PRD_DIRECT * 4];
u32 prdt_indirect[(SATA_FSL_MAX_PRD - SATA_FSL_MAX_PRD_DIRECT) * 4];
};
@@ -531,8 +533,8 @@ static enum ata_completion_errors sata_fsl_qc_prep(struct ata_queued_cmd *qc)
/* setup "ACMD - atapi command" in cmd. desc. if this is ATAPI cmd */
if (ata_is_atapi(qc->tf.protocol)) {
desc_info |= ATAPI_CMD;
- memset((void *)&cd->acmd, 0, 32);
- memcpy((void *)&cd->acmd, qc->cdb, qc->dev->cdb_len);
+ memset(&cd->cdb, 0, sizeof(cd->cdb));
+ memcpy(&cd->cdb, qc->cdb, qc->dev->cdb_len);
}
if (qc->flags & ATA_QCFLAG_DMAMAP)
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (4 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 05/17] sata_fsl: " Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-14 3:56 ` kernel test robot
` (6 more replies)
2021-12-13 22:33 ` [PATCH 07/17] fortify: Detect struct member overflows in memmove() " Kees Cook
` (12 subsequent siblings)
18 siblings, 7 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, linux-kernel
memcpy() is dead; long live memcpy()
tl;dr: In order to eliminate a large class of common buffer overflow
flaws that continue to persist in the kernel, have memcpy() (under
CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct
member when they have a known size. This would have caught all of the
memcpy()-related buffer write overflow flaws identified in at least the
last three years.
Background and analysis:
While stack-based buffer overflow flaws are largely mitigated by stack
canaries (and similar) features, heap-based buffer overflow flaws continue
to regularly appear in the kernel. Many classes of heap buffer overflows
are mitigated by FORTIFY_SOURCE when using the strcpy() family of
functions, but a significant number remain exposed through the memcpy()
family of functions.
At its core, FORTIFY_SOURCE uses the compiler's __builtin_object_size()
internal[0] to determine the available size at a target address based on
the compile-time known structure layout details. It operates in two
modes: outer bounds (0) and inner bounds (1). In mode 0, the size of the
enclosing structure is used. In mode 1, the size of the specific field
is used. For example:
struct object {
u16 scalar1; /* 2 bytes */
char array[6]; /* 6 bytes */
u64 scalar2; /* 8 bytes */
u32 scalar3; /* 4 bytes */
u32 scalar4; /* 4 bytes */
} instance;
__builtin_object_size(instance.array, 0) == 22, since the remaining size
of the enclosing structure starting from "array" is 22 bytes (6 + 8 +
4 + 4).
__builtin_object_size(instance.array, 1) == 6, since the remaining size
of the specific field "array" is 6 bytes.
The initial implementation of FORTIFY_SOURCE used mode 0 because there
were many cases of both strcpy() and memcpy() functions being used to
write (or read) across multiple fields in a structure. For example,
it would catch this, which is writing 2 bytes beyond the end of
"instance":
memcpy(&instance.array, data, 24);
While this didn't protect against overwriting adjacent fields in a given
structure, it would at least stop overflows from reaching beyond the
end of the structure into neighboring memory, and provided a meaningful
mitigation of a subset of buffer overflow flaws. However, many desirable
targets remain within the enclosing structure (for example function
pointers).
As it happened, there were very few cases of strcpy() family functions
intentionally writing beyond the end of a string buffer. Once all known
cases were removed from the kernel, the strcpy() family was tightened[1]
to use mode 1, providing greater mitigation coverage.
What remains is switching memcpy() to mode 1 as well, but making the
switch is much more difficult because of how frustrating it can be to
find existing "normal" uses of memcpy() that expect to write (or read)
across multiple fields. The root cause of the problem is that the C
language lacks a common pattern to indicate the intent of an author's
use of memcpy(), and is further complicated by the available compile-time
and run-time mitigation behaviors.
The FORTIFY_SOURCE mitigation comes in two halves: the compile-time half,
when both the buffer size _and_ the length of the copy is known, and the
run-time half, when only the buffer size is known. If neither size is
known, there is no bounds checking possible. At compile-time when the
compiler sees that a length will always exceed a known buffer size,
a warning can be deterministically emitted. For the run-time half,
the length is tested against the known size of the buffer, and the
overflowing operation is detected. (The performance overhead for these
tests is virtually zero.)
It is relatively easy to find compile-time false-positives since a warning
is always generated. Fixing the false positives, however, can be very
time-consuming as there are hundreds of instances. While it's possible
some over-read conditions could lead to kernel memory exposures, the bulk
of the risk comes from the run-time flaws where the length of a write
may end up being attacker-controlled and lead to an overflow.
Many of the compile-time false-positives take a form similar to this:
memcpy(&instance.scalar2, data, sizeof(instance.scalar2) +
sizeof(instance.scalar3));
and the run-time ones are similar, but lack a constant expression for the
size of the copy:
memcpy(instance.array, data, length);
The former is meant to cover multiple fields (though its style has been
frowned upon more recently), but has been technically legal. Both lack
any expressivity in the C language about the author's _intent_ in a way
that a compiler can check when the length isn't known at compile time.
A comment doesn't work well because what's needed is something a compiler
can directly reason about. Is a given memcpy() call expected to overflow
into neighbors? Is it not? By using the new struct_group() macro, this
intent can be much more easily encoded.
It is not as easy to find the run-time false-positives since the code path
to exercise a seemingly out-of-bounds condition that is actually expected
may not be trivially reachable. Tightening the restrictions to block an
operation for a false positive will either potentially create a greater
flaw (if a copy is truncated by the mitigation), or destabilize the kernel
(e.g. with a BUG()), making things completely useless for the end user.
As a result, tightening the memcpy() restriction (when there is a
reasonable level of uncertainty of the number of false positives), needs
to first WARN() with no truncation. (Though any sufficiently paranoid
end-user can always opt to set the panic_on_warn=1 sysctl.) Once enough
development time has passed, the mitigation can be further intensified.
Given the potential frustrations of weeding out all the false positives
when tightening the run-time checks, it is reasonable to wonder if these
changes would actually add meaningful protection. Looking at just the
last three years, there are 23 identified flaws with a CVE that mention
"buffer overflow", and 11 are memcpy()-related buffer overflows.
(For the remaining 12: 7 are array index overflows that would be
mitigated by systems built with CONFIG_UBSAN_BOUNDS=y: CVE-2019-0145,
CVE-2019-14835, CVE-2019-14896, CVE-2019-14897, CVE-2019-14901,
CVE-2019-17666, CVE-2021-28952. 2 are miscalculated allocation
sizes which could be mitigated with memory tagging: CVE-2019-16746,
CVE-2019-2181. 1 is an iovec buffer bug maybe mitigated by memory tagging:
CVE-2020-10742. 1 is a type confusion bug mitigated by stack canaries:
CVE-2020-10942. 1 is a string handling logic bug with no mitigation I'm
aware of: CVE-2021-28972.)
At my last count on an x86_64 allmodconfig build, there are 25,018 calls
to memcpy(). With callers instrumented to report all places where the
buffer size is known but the length remains unknown (i.e. a run-time
bounds check is added), we can count how many new run-time bounds checks
are added when the destination and source arguments of memcpy() are
changed to use "mode 1" bounds checking: 1540. In addition, there were
146 new compile-time warnings to evaluate and fix.
With this it's also possible to compare the places where the known 11
memcpy() flaw overflows happened against the resulting list of potential
new bounds checks, as a measure of potential efficacy of the tightened
mitigation. Much to my surprise, horror, and delight, all 11 flaws would
have been detected by the newly added run-time bounds checks, making this
a distinctly clear mitigation improvement: 100% coverage for memcpy()
flaws, with a possible 2 orders of magnitude gain in coverage over
existing but undiscovered run-time dynamic length flaws, against only 6%
of all callers maybe gaining a false positive run-time check, with fewer
than 150 new compile-time instances needing evaluation.
Specifically these would have been mitigated:
CVE-2020-24490 https://git.kernel.org/linus/a2ec905d1e160a33b2e210e45ad30445ef26ce0e
CVE-2020-12654 https://git.kernel.org/linus/3a9b153c5591548612c3955c9600a98150c81875
CVE-2020-12653 https://git.kernel.org/linus/b70261a288ea4d2f4ac7cd04be08a9f0f2de4f4d
CVE-2019-14895 https://git.kernel.org/linus/3d94a4a8373bf5f45cf5f939e88b8354dbf2311b
CVE-2019-14816 https://git.kernel.org/linus/7caac62ed598a196d6ddf8d9c121e12e082cac3a
CVE-2019-14815 https://git.kernel.org/linus/7caac62ed598a196d6ddf8d9c121e12e082cac3a
CVE-2019-14814 https://git.kernel.org/linus/7caac62ed598a196d6ddf8d9c121e12e082cac3a
CVE-2019-10126 https://git.kernel.org/linus/69ae4f6aac1578575126319d3f55550e7e440449
CVE-2019-9500 https://git.kernel.org/linus/1b5e2423164b3670e8bc9174e4762d297990deff
no-CVE-yet https://git.kernel.org/linus/130f634da1af649205f4a3dd86cbe5c126b57914
no-CVE-yet https://git.kernel.org/linus/d10a87a3535cce2b890897914f5d0d83df669c63
To accelerate the review of potential run-time false positives, it's
also worth noting that it is possible to partially automate checking
by examining the memcpy() buffer argument to check for the destination
struct member having a neighboring array member. It is reasonable to
expect that the vast majority of run-time false positives would look like
the already evaluated and fixed compile-time false positives, where the
most common pattern is neighboring arrays. (And, FWIW, several of the
compile-time fixes were actual bugs.)
Implementation:
Tighten the memcpy() destination buffer size checking to use the actual
("mode 1") target buffer size as the bounds check instead of their
enclosing structure's ("mode 0") size. Use a common inline for memcpy()
(and memmove() in a following patch), since all the tests are the
same. All new cross-field memcpy() uses must use the struct_group() macro
or similar to target a specific range of fields, so that FORTIFY_SOURCE
can reason about the size and safety of the copy.
For now, cross-member "mode 1" read detection at compile-time will be
limited to W=1 builds, since it is, unfortunately, very common. As the
priority is solving write overflows, read overflows can be part of the
next phase.
For run-time, the "mode 0" size checking and mitigation is left unchanged,
with "mode 1" to be added in stages. In this patch, no new run-time
checks are added. Future patches will first bounds-check writes,
and only perform a WARN() for now. This way any missed run-time false
positives can be flushed out over the coming several development cycles,
but system builders who have tested their workloads to be WARN()-free
can enable the panic_on_warn=1 sysctl to immediately gain a mitigation
against this class of buffer overflows. Once that is under way, run-time
bounds-checking of reads can be similarly added.
Related classes of flaws that remain unmitigated:
- memcpy() with raw pointers (e.g. void *, char *, etc) have no good
mitigation beyond memory tagging (and even that would only protect
against inter-object overflow, not intra-object neighboring field
overflows). Some kind of "fat pointer" solution is likely needed to
gain proper size-of-buffer awareness.
- type confusion where a higher level type's allocation size does
not match the resulting cast type eventually passed to a deeper
memcpy() call where the compiler cannot see the true type. In
theory, greater static analysis could catch these.
[0] https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html
[1] https://git.kernel.org/linus/6a39e62abbafd1d58d1722f40c7d26ef379c6a2f
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 109 ++++++++++++++++--
lib/Makefile | 3 +-
lib/string_helpers.c | 6 +
.../read_overflow2_field-memcpy.c | 5 +
.../write_overflow_field-memcpy.c | 5 +
5 files changed, 115 insertions(+), 13 deletions(-)
create mode 100644 lib/test_fortify/read_overflow2_field-memcpy.c
create mode 100644 lib/test_fortify/write_overflow_field-memcpy.c
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index a6cd6815f249..132efa1ff49f 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -8,7 +8,9 @@
void fortify_panic(const char *name) __noreturn __cold;
void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
+void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)");
+void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");
#define __compiletime_strlen(p) \
({ \
@@ -209,22 +211,105 @@ __FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
return __underlying_memset(p, c, size);
}
-__FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
+/*
+ * To make sure the compiler can enforce protection against buffer overflows,
+ * memcpy(), memmove(), and memset() must not be used beyond individual
+ * struct members. If you need to copy across multiple members, please use
+ * struct_group() to create a named mirror of an anonymous struct union.
+ * (e.g. see struct sk_buff.)
+ *
+ * Mitigation coverage
+ * Bounds checking at:
+ * +-------+-------+-------+-------+
+ * | Compile time | Run time |
+ * memcpy() argument sizes: | write | read | write | read |
+ * +-------+-------+-------+-------+
+ * memcpy(known, known, constant) | y | y | n/a | n/a |
+ * memcpy(unknown, known, constant) | n | y | V | n/a |
+ * memcpy(known, unknown, constant) | y | n | n/a | V |
+ * memcpy(unknown, unknown, constant) | n | n | V | V |
+ * memcpy(known, known, dynamic) | n | n | b | B |
+ * memcpy(unknown, known, dynamic) | n | n | V | B |
+ * memcpy(known, unknown, dynamic) | n | n | b | V |
+ * memcpy(unknown, unknown, dynamic) | n | n | V | V |
+ * +-------+-------+-------+-------+
+ *
+ * y = deterministic compile-time bounds checking
+ * n = cannot do deterministic compile-time bounds checking
+ * n/a = no run-time bounds checking needed since compile-time deterministic
+ * b = perform run-time bounds checking
+ * B = can perform run-time bounds checking, but current unenforced
+ * V = vulnerable to run-time overflow
+ *
+ */
+__FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
+ const size_t p_size,
+ const size_t q_size,
+ const size_t p_size_field,
+ const size_t q_size_field,
+ const char *func)
{
- size_t p_size = __builtin_object_size(p, 0);
- size_t q_size = __builtin_object_size(q, 0);
-
if (__builtin_constant_p(size)) {
- if (p_size < size)
+ /*
+ * Length argument is a constant expression, so we
+ * can perform compile-time bounds checking where
+ * buffer sizes are known.
+ */
+
+ /* Error when size is larger than enclosing struct. */
+ if (p_size > p_size_field && p_size < size)
__write_overflow();
- if (q_size < size)
+ if (q_size > q_size_field && q_size < size)
__read_overflow2();
+
+ /* Warn when write size argument larger than dest field. */
+ if (p_size_field < size)
+ __write_overflow_field(p_size_field, size);
+ /*
+ * Warn for source field over-read when building with W=1
+ * or when an over-write happened, so both can be fixed at
+ * the same time.
+ */
+ if ((IS_ENABLED(KBUILD_EXTRA_WARN1) || p_size_field < size) &&
+ q_size_field < size)
+ __read_overflow2_field(q_size_field, size);
}
- if (p_size < size || q_size < size)
- fortify_panic(__func__);
- return __underlying_memcpy(p, q, size);
+ /*
+ * At this point, length argument may not be a constant expression,
+ * so run-time bounds checking can be done where buffer sizes are
+ * known. (This is not an "else" because the above checks may only
+ * be compile-time warnings, and we want to still warn for run-time
+ * overflows.)
+ */
+
+ /*
+ * Always stop accesses beyond the struct that contains the
+ * field, when the buffer's remaining size is known.
+ * (The -1 test is to optimize away checks where the buffer
+ * lengths are unknown.)
+ */
+ if ((p_size != (size_t)(-1) && p_size < size) ||
+ (q_size != (size_t)(-1) && q_size < size))
+ fortify_panic(func);
}
+#define __fortify_memcpy_chk(p, q, size, p_size, q_size, \
+ p_size_field, q_size_field, op) ({ \
+ size_t __fortify_size = (size_t)(size); \
+ fortify_memcpy_chk(__fortify_size, p_size, q_size, \
+ p_size_field, q_size_field, #op); \
+ __underlying_##op(p, q, __fortify_size); \
+})
+
+/*
+ * __builtin_object_size() must be captured here to avoid evaluating argument
+ * side-effects further into the macro layers.
+ */
+#define memcpy(p, q, s) __fortify_memcpy_chk(p, q, s, \
+ __builtin_object_size(p, 0), __builtin_object_size(q, 0), \
+ __builtin_object_size(p, 1), __builtin_object_size(q, 1), \
+ memcpy)
+
__FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
{
size_t p_size = __builtin_object_size(p, 0);
@@ -304,13 +389,14 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
return __real_kmemdup(p, size, gfp);
}
-/* defined after fortified strlen and memcpy to reuse them */
+/* Defined after fortified strlen to reuse it. */
__FORTIFY_INLINE char *strcpy(char *p, const char *q)
{
size_t p_size = __builtin_object_size(p, 1);
size_t q_size = __builtin_object_size(q, 1);
size_t size;
+ /* If neither buffer size is known, immediately give up. */
if (p_size == (size_t)-1 && q_size == (size_t)-1)
return __underlying_strcpy(p, q);
size = strlen(q) + 1;
@@ -320,14 +406,13 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
/* Run-time check for dynamic size overflow. */
if (p_size < size)
fortify_panic(__func__);
- memcpy(p, q, size);
+ __underlying_memcpy(p, q, size);
return p;
}
/* Don't use these outside the FORITFY_SOURCE implementation */
#undef __underlying_memchr
#undef __underlying_memcmp
-#undef __underlying_memcpy
#undef __underlying_memmove
#undef __underlying_memset
#undef __underlying_strcat
diff --git a/lib/Makefile b/lib/Makefile
index b213a7bbf3fd..7ea43029431b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -373,7 +373,8 @@ TEST_FORTIFY_LOG = test_fortify.log
quiet_cmd_test_fortify = TEST $@
cmd_test_fortify = $(CONFIG_SHELL) $(srctree)/scripts/test_fortify.sh \
$< $@ "$(NM)" $(CC) $(c_flags) \
- $(call cc-disable-warning,fortify-source)
+ $(call cc-disable-warning,fortify-source) \
+ -DKBUILD_EXTRA_WARN1
targets += $(TEST_FORTIFY_LOGS)
clean-files += $(TEST_FORTIFY_LOGS)
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 90f9f1b7afec..4f877e9551d5 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -968,6 +968,12 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
EXPORT_SYMBOL(memcpy_and_pad);
#ifdef CONFIG_FORTIFY_SOURCE
+/* These are placeholders for fortify compile-time warnings. */
+void __read_overflow2_field(size_t avail, size_t wanted) { }
+EXPORT_SYMBOL(__read_overflow2_field);
+void __write_overflow_field(size_t avail, size_t wanted) { }
+EXPORT_SYMBOL(__write_overflow_field);
+
void fortify_panic(const char *name)
{
pr_emerg("detected buffer overflow in %s\n", name);
diff --git a/lib/test_fortify/read_overflow2_field-memcpy.c b/lib/test_fortify/read_overflow2_field-memcpy.c
new file mode 100644
index 000000000000..de9569266223
--- /dev/null
+++ b/lib/test_fortify/read_overflow2_field-memcpy.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST \
+ memcpy(large, instance.buf, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
diff --git a/lib/test_fortify/write_overflow_field-memcpy.c b/lib/test_fortify/write_overflow_field-memcpy.c
new file mode 100644
index 000000000000..28cc81058dd3
--- /dev/null
+++ b/lib/test_fortify/write_overflow_field-memcpy.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST \
+ memcpy(instance.buf, large, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
@ 2021-12-14 3:56 ` kernel test robot
2021-12-14 8:46 ` kernel test robot
` (5 subsequent siblings)
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 3:56 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11370 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: i386-randconfig-s002-20211213 (https://download.01.org/0day-ci/archive/20211214/202112141126.S8eWMmlc-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/80c8d2aae95aae6bd09e3ef84b74d0afe631abae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 80c8d2aae95aae6bd09e3ef84b74d0afe631abae
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/pci/endpoint/functions/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *[assigned] dst_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void const *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] dst_addr
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *[assigned] src_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void const *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] src_addr
>> drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *[assigned] dst_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void const *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] dst_addr
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *[assigned] src_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void const *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] src_addr
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void * @@ got void [noderef] __iomem *[assigned] dst_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] dst_addr
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *[assigned] src_addr @@
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: expected void const *
drivers/pci/endpoint/functions/pci-epf-test.c:288:17: sparse: got void [noderef] __iomem *[assigned] src_addr
vim +288 drivers/pci/endpoint/functions/pci-epf-test.c
5893c2e5353bb9a Kishon Vijay Abraham I 2020-03-16 226
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 227 static int pci_epf_test_copy(struct pci_epf_test *epf_test)
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 228 {
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 229 int ret;
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 230 bool use_dma;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 231 void __iomem *src_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 232 void __iomem *dst_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 233 phys_addr_t src_phys_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 234 phys_addr_t dst_phys_addr;
5893c2e5353bb9a Kishon Vijay Abraham I 2020-03-16 235 struct timespec64 start, end;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 236 struct pci_epf *epf = epf_test->epf;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 237 struct device *dev = &epf->dev;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 238 struct pci_epc *epc = epf->epc;
3235b994950d84d Kishon Vijay Abraham I 2017-08-18 239 enum pci_barno test_reg_bar = epf_test->test_reg_bar;
3235b994950d84d Kishon Vijay Abraham I 2017-08-18 240 struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 241
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 242 src_addr = pci_epc_mem_alloc_addr(epc, &src_phys_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 243 if (!src_addr) {
798c0441bec8c46 Gustavo Pimentel 2018-05-14 244 dev_err(dev, "Failed to allocate source address\n");
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 245 reg->status = STATUS_SRC_ADDR_INVALID;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 246 ret = -ENOMEM;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 247 goto err;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 248 }
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 249
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 250 ret = pci_epc_map_addr(epc, epf->func_no, epf->vfunc_no, src_phys_addr,
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 251 reg->src_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 252 if (ret) {
798c0441bec8c46 Gustavo Pimentel 2018-05-14 253 dev_err(dev, "Failed to map source address\n");
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 254 reg->status = STATUS_SRC_ADDR_INVALID;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 255 goto err_src_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 256 }
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 257
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 258 dst_addr = pci_epc_mem_alloc_addr(epc, &dst_phys_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 259 if (!dst_addr) {
798c0441bec8c46 Gustavo Pimentel 2018-05-14 260 dev_err(dev, "Failed to allocate destination address\n");
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 261 reg->status = STATUS_DST_ADDR_INVALID;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 262 ret = -ENOMEM;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 263 goto err_src_map_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 264 }
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 265
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 266 ret = pci_epc_map_addr(epc, epf->func_no, epf->vfunc_no, dst_phys_addr,
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 267 reg->dst_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 268 if (ret) {
798c0441bec8c46 Gustavo Pimentel 2018-05-14 269 dev_err(dev, "Failed to map destination address\n");
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 270 reg->status = STATUS_DST_ADDR_INVALID;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 271 goto err_dst_addr;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 272 }
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 273
5893c2e5353bb9a Kishon Vijay Abraham I 2020-03-16 274 ktime_get_ts64(&start);
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 275 use_dma = !!(reg->flags & FLAG_USE_DMA);
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 276 if (use_dma) {
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 277 if (!epf_test->dma_supported) {
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 278 dev_err(dev, "Cannot transfer data using DMA\n");
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 279 ret = -EINVAL;
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 280 goto err_map_addr;
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 281 }
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 282
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 283 ret = pci_epf_test_data_transfer(epf_test, dst_phys_addr,
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 284 src_phys_addr, reg->size);
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 285 if (ret)
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 286 dev_err(dev, "Data transfer failed\n");
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 287 } else {
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 @288 memcpy(dst_addr, src_addr, reg->size);
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 289 }
5893c2e5353bb9a Kishon Vijay Abraham I 2020-03-16 290 ktime_get_ts64(&end);
5893c2e5353bb9a Kishon Vijay Abraham I 2020-03-16 291 pci_epf_test_print_rate("COPY", reg->size, &start, &end, use_dma);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 292
5ebf3fc59bd20d1 Kishon Vijay Abraham I 2020-03-16 293 err_map_addr:
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 294 pci_epc_unmap_addr(epc, epf->func_no, epf->vfunc_no, dst_phys_addr);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 295
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 296 err_dst_addr:
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 297 pci_epc_mem_free_addr(epc, dst_phys_addr, dst_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 298
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 299 err_src_map_addr:
53fd3cbe5e9d791 Kishon Vijay Abraham I 2021-08-19 300 pci_epc_unmap_addr(epc, epf->func_no, epf->vfunc_no, src_phys_addr);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 301
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 302 err_src_addr:
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 303 pci_epc_mem_free_addr(epc, src_phys_addr, src_addr, reg->size);
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 304
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 305 err:
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 306 return ret;
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 307 }
349e7a85b25fa6e Kishon Vijay Abraham I 2017-03-27 308
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-12-14 3:56 ` kernel test robot
@ 2021-12-14 8:46 ` kernel test robot
2021-12-14 11:50 ` kernel test robot
` (4 subsequent siblings)
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 8:46 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 7981 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: x86_64-randconfig-s022-20211213 (https://download.01.org/0day-ci/archive/20211214/202112141639.6sg1Fhdq-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/80c8d2aae95aae6bd09e3ef84b74d0afe631abae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 80c8d2aae95aae6bd09e3ef84b74d0afe631abae
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/acpi/apei/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/acpi/apei/erst.c:272:13: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *src @@ got void [noderef] __iomem * @@
drivers/acpi/apei/erst.c:272:13: sparse: expected void *src
drivers/acpi/apei/erst.c:272:13: sparse: got void [noderef] __iomem *
drivers/acpi/apei/erst.c:275:13: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *dst @@ got void [noderef] __iomem * @@
drivers/acpi/apei/erst.c:275:13: sparse: expected void *dst
drivers/acpi/apei/erst.c:275:13: sparse: got void [noderef] __iomem *
drivers/acpi/apei/erst.c:277:25: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got void *src @@
drivers/acpi/apei/erst.c:277:25: sparse: expected void volatile [noderef] __iomem *addr
drivers/acpi/apei/erst.c:277:25: sparse: got void *src
drivers/acpi/apei/erst.c:283:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got void *src @@
drivers/acpi/apei/erst.c:283:17: sparse: expected void volatile [noderef] __iomem *addr
drivers/acpi/apei/erst.c:283:17: sparse: got void *src
drivers/acpi/apei/erst.c:284:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got void *dst @@
drivers/acpi/apei/erst.c:284:17: sparse: expected void volatile [noderef] __iomem *addr
drivers/acpi/apei/erst.c:284:17: sparse: got void *dst
>> drivers/acpi/apei/erst.c:792:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *static [toplevel] vaddr @@
drivers/acpi/apei/erst.c:792:9: sparse: expected void const *
drivers/acpi/apei/erst.c:792:9: sparse: got void [noderef] __iomem *static [toplevel] vaddr
>> drivers/acpi/apei/erst.c:792:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *static [toplevel] vaddr @@
drivers/acpi/apei/erst.c:792:9: sparse: expected void const *
drivers/acpi/apei/erst.c:792:9: sparse: got void [noderef] __iomem *static [toplevel] vaddr
drivers/acpi/apei/erst.c:792:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void * @@ got void [noderef] __iomem *static [toplevel] vaddr @@
drivers/acpi/apei/erst.c:792:9: sparse: expected void *
drivers/acpi/apei/erst.c:792:9: sparse: got void [noderef] __iomem *static [toplevel] vaddr
drivers/acpi/apei/erst.c:793:20: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cper_record_header *rcd_erange @@ got void [noderef] __iomem *static [toplevel] vaddr @@
drivers/acpi/apei/erst.c:793:20: sparse: expected struct cper_record_header *rcd_erange
drivers/acpi/apei/erst.c:793:20: sparse: got void [noderef] __iomem *static [toplevel] vaddr
drivers/acpi/apei/erst.c:830:17: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected struct cper_record_header *rcd_tmp @@ got void [noderef] __iomem * @@
drivers/acpi/apei/erst.c:830:17: sparse: expected struct cper_record_header *rcd_tmp
drivers/acpi/apei/erst.c:830:17: sparse: got void [noderef] __iomem *
vim +792 drivers/acpi/apei/erst.c
a08f82d08053fb Huang Ying 2010-05-18 766
a08f82d08053fb Huang Ying 2010-05-18 767 int erst_write(const struct cper_record_header *record)
a08f82d08053fb Huang Ying 2010-05-18 768 {
a08f82d08053fb Huang Ying 2010-05-18 769 int rc;
a08f82d08053fb Huang Ying 2010-05-18 770 unsigned long flags;
a08f82d08053fb Huang Ying 2010-05-18 771 struct cper_record_header *rcd_erange;
a08f82d08053fb Huang Ying 2010-05-18 772
a08f82d08053fb Huang Ying 2010-05-18 773 if (erst_disable)
a08f82d08053fb Huang Ying 2010-05-18 774 return -ENODEV;
a08f82d08053fb Huang Ying 2010-05-18 775
a08f82d08053fb Huang Ying 2010-05-18 776 if (memcmp(record->signature, CPER_SIG_RECORD, CPER_SIG_SIZE))
a08f82d08053fb Huang Ying 2010-05-18 777 return -EINVAL;
a08f82d08053fb Huang Ying 2010-05-18 778
a08f82d08053fb Huang Ying 2010-05-18 779 if (erst_erange.attr & ERST_RANGE_NVRAM) {
3b38bb5f7f0635 Huang Ying 2010-12-02 780 if (!raw_spin_trylock_irqsave(&erst_lock, flags))
a08f82d08053fb Huang Ying 2010-05-18 781 return -EBUSY;
a08f82d08053fb Huang Ying 2010-05-18 782 rc = __erst_write_to_nvram(record);
3b38bb5f7f0635 Huang Ying 2010-12-02 783 raw_spin_unlock_irqrestore(&erst_lock, flags);
a08f82d08053fb Huang Ying 2010-05-18 784 return rc;
a08f82d08053fb Huang Ying 2010-05-18 785 }
a08f82d08053fb Huang Ying 2010-05-18 786
a08f82d08053fb Huang Ying 2010-05-18 787 if (record->record_length > erst_erange.size)
a08f82d08053fb Huang Ying 2010-05-18 788 return -EINVAL;
a08f82d08053fb Huang Ying 2010-05-18 789
3b38bb5f7f0635 Huang Ying 2010-12-02 790 if (!raw_spin_trylock_irqsave(&erst_lock, flags))
a08f82d08053fb Huang Ying 2010-05-18 791 return -EBUSY;
a08f82d08053fb Huang Ying 2010-05-18 @792 memcpy(erst_erange.vaddr, record, record->record_length);
a08f82d08053fb Huang Ying 2010-05-18 793 rcd_erange = erst_erange.vaddr;
a08f82d08053fb Huang Ying 2010-05-18 794 /* signature for serialization system */
a08f82d08053fb Huang Ying 2010-05-18 795 memcpy(&rcd_erange->persistence_information, "ER", 2);
a08f82d08053fb Huang Ying 2010-05-18 796
a08f82d08053fb Huang Ying 2010-05-18 797 rc = __erst_write_to_storage(0);
3b38bb5f7f0635 Huang Ying 2010-12-02 798 raw_spin_unlock_irqrestore(&erst_lock, flags);
a08f82d08053fb Huang Ying 2010-05-18 799
a08f82d08053fb Huang Ying 2010-05-18 800 return rc;
a08f82d08053fb Huang Ying 2010-05-18 801 }
a08f82d08053fb Huang Ying 2010-05-18 802 EXPORT_SYMBOL_GPL(erst_write);
a08f82d08053fb Huang Ying 2010-05-18 803
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-12-14 3:56 ` kernel test robot
2021-12-14 8:46 ` kernel test robot
@ 2021-12-14 11:50 ` kernel test robot
2021-12-14 16:32 ` kernel test robot
` (3 subsequent siblings)
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 11:50 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 11202 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: riscv-randconfig-s031-20211214 (https://download.01.org/0day-ci/archive/20211214/202112141904.hqRtWx0l-lkp(a)intel.com/config)
compiler: riscv32-linux-gcc (GCC) 11.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/80c8d2aae95aae6bd09e3ef84b74d0afe631abae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 80c8d2aae95aae6bd09e3ef84b74d0afe631abae
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=riscv SHELL=/bin/bash drivers/video/fbdev/core/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/video/fbdev/core/fbmem.c:808:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] src @@
drivers/video/fbdev/core/fbmem.c:808:17: sparse: expected void const *
drivers/video/fbdev/core/fbmem.c:808:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] src
drivers/video/fbdev/core/fbmem.c:808:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] src @@
drivers/video/fbdev/core/fbmem.c:808:17: sparse: expected void const *
drivers/video/fbdev/core/fbmem.c:808:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] src
drivers/video/fbdev/core/fbmem.c:808:17: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] src @@
drivers/video/fbdev/core/fbmem.c:808:17: sparse: expected void const *
drivers/video/fbdev/core/fbmem.c:808:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] src
>> drivers/video/fbdev/core/fbmem.c:885:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] dst @@
drivers/video/fbdev/core/fbmem.c:885:17: sparse: expected void const *
drivers/video/fbdev/core/fbmem.c:885:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] dst
>> drivers/video/fbdev/core/fbmem.c:885:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] dst @@
drivers/video/fbdev/core/fbmem.c:885:17: sparse: expected void const *
drivers/video/fbdev/core/fbmem.c:885:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] dst
drivers/video/fbdev/core/fbmem.c:885:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void * @@ got unsigned char [noderef] [usertype] __iomem *[assigned] dst @@
drivers/video/fbdev/core/fbmem.c:885:17: sparse: expected void *
drivers/video/fbdev/core/fbmem.c:885:17: sparse: got unsigned char [noderef] [usertype] __iomem *[assigned] dst
vim +885 drivers/video/fbdev/core/fbmem.c
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 826
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 827 static ssize_t
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 828 fb_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 829 {
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 830 unsigned long p = *ppos;
c47747fde931c0 drivers/video/fbmem.c Linus Torvalds 2011-05-11 831 struct fb_info *info = file_fb_info(file);
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 832 u8 *buffer, *src;
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 833 u8 __iomem *dst;
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 834 int c, cnt = 0, err = 0;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 835 unsigned long total_size;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 836
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 837 if (!info || !info->screen_base)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 838 return -ENODEV;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 839
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 840 if (info->state != FBINFO_STATE_RUNNING)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 841 return -EPERM;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 842
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 843 if (info->fbops->fb_write)
3f9b0880e4a96b drivers/video/fbmem.c Antonino A. Daplas 2007-05-08 844 return info->fbops->fb_write(info, buf, count, ppos);
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 845
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 846 total_size = info->screen_size;
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 847
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 848 if (total_size == 0)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 849 total_size = info->fix.smem_len;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 850
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 851 if (p > total_size)
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 852 return -EFBIG;
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 853
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 854 if (count > total_size) {
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 855 err = -EFBIG;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 856 count = total_size;
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 857 }
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 858
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 859 if (count + p > total_size) {
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 860 if (!err)
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 861 err = -ENOSPC;
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 862
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 863 count = total_size - p;
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 864 }
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 865
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 866 buffer = kmalloc((count > PAGE_SIZE) ? PAGE_SIZE : count,
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 867 GFP_KERNEL);
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 868 if (!buffer)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 869 return -ENOMEM;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 870
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 871 dst = (u8 __iomem *) (info->screen_base + p);
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 872
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 873 if (info->fbops->fb_sync)
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 874 info->fbops->fb_sync(info);
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 875
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 876 while (count) {
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 877 c = (count > PAGE_SIZE) ? PAGE_SIZE : count;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 878 src = buffer;
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 879
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 880 if (copy_from_user(src, buf, c)) {
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 881 err = -EFAULT;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 882 break;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 883 }
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 884
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 @885 fb_memcpy_tofb(dst, src, c);
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 886 dst += c;
f11b478d461b71 drivers/video/fbmem.c James Hogan 2010-10-27 887 src += c;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 888 *ppos += c;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 889 buf += c;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 890 cnt += c;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 891 count -= c;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 892 }
0a484a3af905a2 drivers/video/fbmem.c Antonino A. Daplas 2006-01-09 893
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 894 kfree(buffer);
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 895
6a2a88668e90cd drivers/video/fbmem.c Antonino A. Daplas 2006-04-18 896 return (cnt) ? cnt : err;
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 897 }
^1da177e4c3f41 drivers/video/fbmem.c Linus Torvalds 2005-04-16 898
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
` (2 preceding siblings ...)
2021-12-14 11:50 ` kernel test robot
@ 2021-12-14 16:32 ` kernel test robot
2021-12-14 19:06 ` kernel test robot
` (2 subsequent siblings)
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 16:32 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 29532 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5 next-20211213]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: i386-randconfig-s002-20211214 (https://download.01.org/0day-ci/archive/20211215/202112150059.yHZrJY0I-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/80c8d2aae95aae6bd09e3ef84b74d0afe631abae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 80c8d2aae95aae6bd09e3ef84b74d0afe631abae
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/video/fbdev/geode/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: got void [noderef] __iomem *gp_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: got void [noderef] __iomem *gp_regs
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:602:9: sparse: got void [noderef] __iomem *gp_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: got void [noderef] __iomem *dc_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: got void [noderef] __iomem *dc_regs
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:603:9: sparse: got void [noderef] __iomem *dc_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: got void [noderef] __iomem *vp_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: got void [noderef] __iomem *vp_regs
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vp_regs @@
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:604:9: sparse: got void [noderef] __iomem *vp_regs
>> drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: got void [noderef] __iomem *
>> drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:605:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:633:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: expected void const *
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: got void [noderef] __iomem *
>> drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: expected void *
drivers/video/fbdev/geode/lxfb_ops.c:760:9: sparse: got void [noderef] __iomem *
vim +602 drivers/video/fbdev/geode/lxfb_ops.c
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 582
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 583 static void lx_save_regs(struct lxfb_par *par)
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 584 {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 585 uint32_t filt;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 586 int i;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 587
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 588 /* wait for the BLT engine to stop being busy */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 589 do {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 590 i = read_gp(par, GP_BLT_STATUS);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 591 } while ((i & GP_BLT_STATUS_PB) || !(i & GP_BLT_STATUS_CE));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 592
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 593 /* save MSRs */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 594 rdmsrl(MSR_LX_MSR_PADSEL, par->msr.padsel);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 595 rdmsrl(MSR_GLCP_DOTPLL, par->msr.dotpll);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 596 rdmsrl(MSR_LX_GLD_MSR_CONFIG, par->msr.dfglcfg);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 597 rdmsrl(MSR_LX_SPARE_MSR, par->msr.dcspare);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 598
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 599 write_dc(par, DC_UNLOCK, DC_UNLOCK_UNLOCK);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 600
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 601 /* save registers */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 @602 memcpy(par->gp, par->gp_regs, sizeof(par->gp));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 @603 memcpy(par->dc, par->dc_regs, sizeof(par->dc));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 @604 memcpy(par->vp, par->vp_regs, sizeof(par->vp));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 @605 memcpy(par->fp, par->vp_regs + VP_FP_START, sizeof(par->fp));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 606
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 607 /* save the display controller palette */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 608 write_dc(par, DC_PAL_ADDRESS, 0);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 609 for (i = 0; i < ARRAY_SIZE(par->dc_pal); i++)
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 610 par->dc_pal[i] = read_dc(par, DC_PAL_DATA);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 611
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 612 /* save the video processor palette */
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 613 write_vp(par, VP_PAR, 0);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 614 for (i = 0; i < ARRAY_SIZE(par->vp_pal); i++)
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 615 par->vp_pal[i] = read_vp(par, VP_PDR);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 616
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 617 /* save the horizontal filter coefficients */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 618 filt = par->dc[DC_IRQ_FILT_CTL] | DC_IRQ_FILT_CTL_H_FILT_SEL;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 619 for (i = 0; i < ARRAY_SIZE(par->hcoeff); i += 2) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 620 write_dc(par, DC_IRQ_FILT_CTL, (filt & 0xffffff00) | i);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 621 par->hcoeff[i] = read_dc(par, DC_FILT_COEFF1);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 622 par->hcoeff[i + 1] = read_dc(par, DC_FILT_COEFF2);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 623 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 624
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 625 /* save the vertical filter coefficients */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 626 filt &= ~DC_IRQ_FILT_CTL_H_FILT_SEL;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 627 for (i = 0; i < ARRAY_SIZE(par->vcoeff); i++) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 628 write_dc(par, DC_IRQ_FILT_CTL, (filt & 0xffffff00) | i);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 629 par->vcoeff[i] = read_dc(par, DC_FILT_COEFF1);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 630 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 631
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 632 /* save video coeff ram */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 633 memcpy(par->vp_coeff, par->vp_regs + VP_VCR, sizeof(par->vp_coeff));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 634 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 635
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 636 static void lx_restore_gfx_proc(struct lxfb_par *par)
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 637 {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 638 int i;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 639
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 640 /* a bunch of registers require GP_RASTER_MODE to be set first */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 641 write_gp(par, GP_RASTER_MODE, par->gp[GP_RASTER_MODE]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 642
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 643 for (i = 0; i < ARRAY_SIZE(par->gp); i++) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 644 switch (i) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 645 case GP_RASTER_MODE:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 646 case GP_VECTOR_MODE:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 647 case GP_BLT_MODE:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 648 case GP_BLT_STATUS:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 649 case GP_HST_SRC:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 650 /* FIXME: restore LUT data */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 651 case GP_LUT_INDEX:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 652 case GP_LUT_DATA:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 653 /* don't restore these registers */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 654 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 655
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 656 default:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 657 write_gp(par, i, par->gp[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 658 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 659 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 660 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 661
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 662 static void lx_restore_display_ctlr(struct lxfb_par *par)
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 663 {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 664 uint32_t filt;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 665 int i;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 666
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 667 wrmsrl(MSR_LX_SPARE_MSR, par->msr.dcspare);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 668
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 669 for (i = 0; i < ARRAY_SIZE(par->dc); i++) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 670 switch (i) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 671 case DC_UNLOCK:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 672 /* unlock the DC; runs first */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 673 write_dc(par, DC_UNLOCK, DC_UNLOCK_UNLOCK);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 674 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 675
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 676 case DC_GENERAL_CFG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 677 case DC_DISPLAY_CFG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 678 /* disable all while restoring */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 679 write_dc(par, i, 0);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 680 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 681
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 682 case DC_DV_CTL:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 683 /* set all ram to dirty */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 684 write_dc(par, i, par->dc[i] | DC_DV_CTL_CLEAR_DV_RAM);
2c3a1e49696fd05 drivers/video/fbdev/geode/lxfb_ops.c Gustavo A. R. Silva 2020-11-20 685 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 686
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 687 case DC_RSVD_1:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 688 case DC_RSVD_2:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 689 case DC_RSVD_3:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 690 case DC_LINE_CNT:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 691 case DC_PAL_ADDRESS:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 692 case DC_PAL_DATA:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 693 case DC_DFIFO_DIAG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 694 case DC_CFIFO_DIAG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 695 case DC_FILT_COEFF1:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 696 case DC_FILT_COEFF2:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 697 case DC_RSVD_4:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 698 case DC_RSVD_5:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 699 /* don't restore these registers */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 700 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 701
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 702 default:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 703 write_dc(par, i, par->dc[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 704 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 705 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 706
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 707 /* restore the palette */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 708 write_dc(par, DC_PAL_ADDRESS, 0);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 709 for (i = 0; i < ARRAY_SIZE(par->dc_pal); i++)
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 710 write_dc(par, DC_PAL_DATA, par->dc_pal[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 711
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 712 /* restore the horizontal filter coefficients */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 713 filt = par->dc[DC_IRQ_FILT_CTL] | DC_IRQ_FILT_CTL_H_FILT_SEL;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 714 for (i = 0; i < ARRAY_SIZE(par->hcoeff); i += 2) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 715 write_dc(par, DC_IRQ_FILT_CTL, (filt & 0xffffff00) | i);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 716 write_dc(par, DC_FILT_COEFF1, par->hcoeff[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 717 write_dc(par, DC_FILT_COEFF2, par->hcoeff[i + 1]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 718 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 719
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 720 /* restore the vertical filter coefficients */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 721 filt &= ~DC_IRQ_FILT_CTL_H_FILT_SEL;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 722 for (i = 0; i < ARRAY_SIZE(par->vcoeff); i++) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 723 write_dc(par, DC_IRQ_FILT_CTL, (filt & 0xffffff00) | i);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 724 write_dc(par, DC_FILT_COEFF1, par->vcoeff[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 725 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 726 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 727
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 728 static void lx_restore_video_proc(struct lxfb_par *par)
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 729 {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 730 int i;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 731
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 732 wrmsrl(MSR_LX_GLD_MSR_CONFIG, par->msr.dfglcfg);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 733 wrmsrl(MSR_LX_MSR_PADSEL, par->msr.padsel);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 734
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 735 for (i = 0; i < ARRAY_SIZE(par->vp); i++) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 736 switch (i) {
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 737 case VP_VCFG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 738 case VP_DCFG:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 739 case VP_PAR:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 740 case VP_PDR:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 741 case VP_CCS:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 742 case VP_RSVD_0:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 743 /* case VP_VDC: */ /* why should this not be restored? */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 744 case VP_RSVD_1:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 745 case VP_CRC32:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 746 /* don't restore these registers */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 747 break;
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 748
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 749 default:
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 750 write_vp(par, i, par->vp[i]);
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 751 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 752 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 753
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 754 /* restore video processor palette */
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 755 write_vp(par, VP_PAR, 0);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 756 for (i = 0; i < ARRAY_SIZE(par->vp_pal); i++)
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 757 write_vp(par, VP_PDR, par->vp_pal[i]);
16cdd4336fdca9b drivers/video/geode/lxfb_ops.c Daniel Drake 2010-11-30 758
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 759 /* restore video coeff ram */
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 @760 memcpy(par->vp_regs + VP_VCR, par->vp_coeff, sizeof(par->vp_coeff));
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 761 }
f694e53bd0db695 drivers/video/geode/lxfb_ops.c Andres Salomon 2008-04-28 762
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
` (3 preceding siblings ...)
2021-12-14 16:32 ` kernel test robot
@ 2021-12-14 19:06 ` kernel test robot
2021-12-16 8:56 ` kernel test robot
2021-12-16 11:08 ` Mark Rutland
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 19:06 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5 next-20211213]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: i386-randconfig-m021-20211214 (https://download.01.org/0day-ci/archive/20211215/202112150326.Cu6pK9ir-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
New smatch warnings:
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3289 pvr2_hdw_get_tuner_status() warn: inconsistent indenting
Old smatch warnings:
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1730 pvr2_hdw_set_streaming() warn: inconsistent indenting
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3461 pvr2_hdw_cpufw_set_enabled() warn: inconsistent indenting
drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3501 pvr2_hdw_cpufw_get() warn: inconsistent indenting
vim +3289 drivers/media/usb/pvrusb2/pvrusb2-hdw.c
432907f750b27aa drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2008-08-31 3279
432907f750b27aa drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2008-08-31 3280
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3281 /* Return information about the tuner */
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3282 int pvr2_hdw_get_tuner_status(struct pvr2_hdw *hdw,struct v4l2_tuner *vtp)
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3283 {
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3284 LOCK_TAKE(hdw->big_lock); do {
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3285 if (hdw->tuner_signal_stale) {
a51f5000b791003 drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2009-03-06 3286 pvr2_hdw_status_poll(hdw);
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3287 }
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3288 memcpy(vtp,&hdw->tuner_signal_info,sizeof(struct v4l2_tuner));
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 @3289 } while (0); LOCK_GIVE(hdw->big_lock);
18103c57b0168eb drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2007-01-20 3290 return 0;
d855497edbfbf9e drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2006-06-26 3291 }
d855497edbfbf9e drivers/media/video/pvrusb2/pvrusb2-hdw.c Mike Isely 2006-06-26 3292
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
` (4 preceding siblings ...)
2021-12-14 19:06 ` kernel test robot
@ 2021-12-16 8:56 ` kernel test robot
2021-12-16 11:08 ` Mark Rutland
6 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-16 8:56 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 9323 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5 next-20211215]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: i386-randconfig-s002-20211216 (https://download.01.org/0day-ci/archive/20211216/202112161632.BTEfICUX-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/80c8d2aae95aae6bd09e3ef84b74d0afe631abae
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 80c8d2aae95aae6bd09e3ef84b74d0afe631abae
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/video/fbdev/geode/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: got void [noderef] __iomem *gp_regs
>> drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: got void [noderef] __iomem *gp_regs
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *gp_regs @@
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:30:9: sparse: got void [noderef] __iomem *gp_regs
>> drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: got void [noderef] __iomem *dc_regs
>> drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: got void [noderef] __iomem *dc_regs
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *dc_regs @@
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:31:9: sparse: got void [noderef] __iomem *dc_regs
>> drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vid_regs @@
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: got void [noderef] __iomem *vid_regs
>> drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vid_regs @@
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: got void [noderef] __iomem *vid_regs
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem *vid_regs @@
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:32:9: sparse: got void [noderef] __iomem *vid_regs
>> drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: got void [noderef] __iomem *
>> drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: got void [noderef] __iomem *
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got void [noderef] __iomem * @@
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: expected void const *
drivers/video/fbdev/geode/suspend_gx.c:33:9: sparse: got void [noderef] __iomem *
vim +30 drivers/video/fbdev/geode/suspend_gx.c
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 13
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 14 static void gx_save_regs(struct gxfb_par *par)
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 15 {
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 16 int i;
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 17
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 18 /* wait for the BLT engine to stop being busy */
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 19 do {
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 20 i = read_gp(par, GP_BLT_STATUS);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 21 } while (i & (GP_BLT_STATUS_BLT_PENDING | GP_BLT_STATUS_BLT_BUSY));
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 22
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 23 /* save MSRs */
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 24 rdmsrl(MSR_GX_MSR_PADSEL, par->msr.padsel);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 25 rdmsrl(MSR_GLCP_DOTPLL, par->msr.dotpll);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 26
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 27 write_dc(par, DC_UNLOCK, DC_UNLOCK_UNLOCK);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 28
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 29 /* save registers */
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 @30 memcpy(par->gp, par->gp_regs, sizeof(par->gp));
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 @31 memcpy(par->dc, par->dc_regs, sizeof(par->dc));
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 @32 memcpy(par->vp, par->vid_regs, sizeof(par->vp));
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 @33 memcpy(par->fp, par->vid_regs + VP_FP_START, sizeof(par->fp));
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 34
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 35 /* save the palette */
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 36 write_dc(par, DC_PAL_ADDRESS, 0);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 37 for (i = 0; i < ARRAY_SIZE(par->pal); i++)
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 38 par->pal[i] = read_dc(par, DC_PAL_DATA);
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 39 }
46fb6f110dfc3f drivers/video/geode/suspend_gx.c Andres Salomon 2008-04-28 40
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
` (5 preceding siblings ...)
2021-12-16 8:56 ` kernel test robot
@ 2021-12-16 11:08 ` Mark Rutland
2021-12-16 11:21 ` Mark Rutland
2021-12-16 18:00 ` Kees Cook
6 siblings, 2 replies; 47+ messages in thread
From: Mark Rutland @ 2021-12-16 11:08 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-hardening, linux-kernel
On Mon, Dec 13, 2021 at 02:33:20PM -0800, Kees Cook wrote:
> memcpy() is dead; long live memcpy()
>
> tl;dr: In order to eliminate a large class of common buffer overflow
> flaws that continue to persist in the kernel, have memcpy() (under
> CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct
> member when they have a known size. This would have caught all of the
> memcpy()-related buffer write overflow flaws identified in at least the
> last three years.
>
Hi Kees,
Since there's a *lot* of context below, it's very easy to miss some key details
(e.g. that the compile-time warnings are limited to W=1 builds). It would be
really nice if the summary above could say something like:
This patch makes it possible to detect when memcpy() of a struct member may
go past the bounds of that member. When CONFIG_FORTIFY_SOURCE=y, runtime
checks are always emitted where the compiler cannot guarantee a memcpy() is
safely bounded, and compile-time warnings are enabled for W=1 builds.
This catches a large class of common buffer overflow flaws, and would have
caught all of the memcpy()-related buffer write overflow flaws identified in
the last three years.
As an aside, since W=1 is chock-full of (IMO useless) warnings, is there any
way to enable *just* the FORTIFY_SOURCE warnings?
[...]
> Implementation:
>
> Tighten the memcpy() destination buffer size checking to use the actual
> ("mode 1") target buffer size as the bounds check instead of their
> enclosing structure's ("mode 0") size. Use a common inline for memcpy()
> (and memmove() in a following patch), since all the tests are the
> same. All new cross-field memcpy() uses must use the struct_group() macro
> or similar to target a specific range of fields, so that FORTIFY_SOURCE
> can reason about the size and safety of the copy.
>
> For now, cross-member "mode 1" read detection at compile-time will be
> limited to W=1 builds, since it is, unfortunately, very common. As the
> priority is solving write overflows, read overflows can be part of the
> next phase.
I had a go at testing this on arm64, and could get build-time warnings from GCC
11.1.0, but not from Clang 13.0.0.
I picked the series from:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=memcpy/step2/next-20211213
I prepped a tree with that branch and a deliberate bug:
| [mark@lakrids:~/src/linux]% git checkout -f kernel-org-kees/memcpy/step2/next-20211213
| [mark@lakrids:~/src/linux]% git clean -qfdx
| [mark@lakrids:~/src/linux]% cat <<EOF >> arch/arm64/kernel/setup.c
|
| struct foo {
| int a;
| int b;
| } foo1, foo2;
|
| void foo_copy(void);
| void foo_copy(void)
| {
| memcpy(&foo1.a, &foo2.a, sizeof(foo1));
| }
| EOF
When building with GCC 11.1.0:
| [mark@lakrids:~/src/linux]% usekorg 11.1.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- -s defconfig
| [mark@lakrids:~/src/linux]% ./scripts/config -e FORTIFY_SOURCE
| [mark@lakrids:~/src/linux]% grep FORTIFY_SOURCE .config
| CONFIG_ARCH_HAS_FORTIFY_SOURCE=y
| CONFIG_FORTIFY_SOURCE=y
| [mark@lakrids:~/src/linux]% usekorg 11.1.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- -s arch/arm64/kernel/setup.o
| In file included from ./include/linux/string.h:253,
| from ./include/linux/bitmap.h:11,
| from ./include/linux/cpumask.h:12,
| from ./include/linux/smp.h:13,
| from ./include/linux/lockdep.h:14,
| from ./include/linux/mutex.h:17,
| from ./include/linux/kernfs.h:12,
| from ./include/linux/sysfs.h:16,
| from ./include/linux/kobject.h:20,
| from ./include/linux/of.h:17,
| from ./include/linux/irqdomain.h:35,
| from ./include/linux/acpi.h:13,
| from arch/arm64/kernel/setup.c:9:
| In function 'fortify_memcpy_chk',
| inlined from 'foo_copy' at arch/arm64/kernel/setup.c:457:8:
| ./include/linux/fortify-string.h:316:25: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
| 316 | __write_overflow_field(p_size_field, size);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| ./include/linux/fortify-string.h:324:25: warning: call to '__read_overflow2_field' declared with attribute warning: detected read beyond size of field (2nd parameter); maybe use struct_group()? [-Wattribute-warning]
| 324 | __read_overflow2_field(q_size_field, size);
| | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
When building with clang 13.0.0 (the vdso bits can be ignored):
| [mark@lakrids:~/src/linux]% usellvm 13.0.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- LLVM=1 -s defconfig
| [mark@lakrids:~/src/linux]% ./scripts/config -e FORTIFY_SOURCE
| [mark@lakrids:~/src/linux]% grep FORTIFY_SOURCE .config
| CONFIG_ARCH_HAS_FORTIFY_SOURCE=y
| CONFIG_FORTIFY_SOURCE=y
| [mark@lakrids:~/src/linux]% usellvm 13.0.0 make ARCH=arm64 CROSS_COMPILE=aarch64-linux- LLVM=1 W=1 -s arch/arm64/kernel/setup.o
| arch/arm64/kernel/vdso/vgettimeofday.c:9:5: warning: no previous prototype for function '__kernel_clock_gettime' [-Wmissing-prototypes]
| int __kernel_clock_gettime(clockid_t clock,
| ^
| arch/arm64/kernel/vdso/vgettimeofday.c:9:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
| int __kernel_clock_gettime(clockid_t clock,
| ^
| static
| arch/arm64/kernel/vdso/vgettimeofday.c:15:5: warning: no previous prototype for function '__kernel_gettimeofday' [-Wmissing-prototypes]
| int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
| ^
| arch/arm64/kernel/vdso/vgettimeofday.c:15:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
| int __kernel_gettimeofday(struct __kernel_old_timeval *tv,
| ^
| static
| arch/arm64/kernel/vdso/vgettimeofday.c:21:5: warning: no previous prototype for function '__kernel_clock_getres' [-Wmissing-prototypes]
| int __kernel_clock_getres(clockid_t clock_id,
| ^
| arch/arm64/kernel/vdso/vgettimeofday.c:21:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
| int __kernel_clock_getres(clockid_t clock_id,
| ^
| static
| 3 warnings generated.
No relevant warnings, but code was generated for runtime warnings:
| 0000000000000000 <foo_copy>:
| 0: d503233f paciasp
| 4: a9bf7bfd stp x29, x30, [sp, #-16]!
| 8: 910003fd mov x29, sp
| c: 52800080 mov w0, #0x4 // #4
| 10: 52800101 mov w1, #0x8 // #8
| 14: 94000000 bl 0 <__write_overflow_field>
| 18: 52800080 mov w0, #0x4 // #4
| 1c: 52800101 mov w1, #0x8 // #8
| 20: 94000000 bl 0 <__read_overflow2_field>
| 24: 90000008 adrp x8, 8 <foo_copy+0x8>
| 28: f9400108 ldr x8, [x8]
| 2c: 90000009 adrp x9, 0 <foo_copy>
| 30: f9000128 str x8, [x9]
| 34: a8c17bfd ldp x29, x30, [sp], #16
| 38: d50323bf autiasp
| 3c: d65f03c0 ret
Have I misunderstood how that's meant to work, or am I doing something wrong?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-16 11:08 ` Mark Rutland
@ 2021-12-16 11:21 ` Mark Rutland
2021-12-16 18:00 ` Kees Cook
1 sibling, 0 replies; 47+ messages in thread
From: Mark Rutland @ 2021-12-16 11:21 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-hardening, linux-kernel
On Thu, Dec 16, 2021 at 11:08:26AM +0000, Mark Rutland wrote:
> On Mon, Dec 13, 2021 at 02:33:20PM -0800, Kees Cook wrote:
> > memcpy() is dead; long live memcpy()
> >
> > tl;dr: In order to eliminate a large class of common buffer overflow
> > flaws that continue to persist in the kernel, have memcpy() (under
> > CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct
> > member when they have a known size. This would have caught all of the
> > memcpy()-related buffer write overflow flaws identified in at least the
> > last three years.
> >
>
> Hi Kees,
>
> Since there's a *lot* of context below, it's very easy to miss some key details
> (e.g. that the compile-time warnings are limited to W=1 builds). It would be
> really nice if the summary above could say something like:
>
> This patch makes it possible to detect when memcpy() of a struct member may
> go past the bounds of that member. When CONFIG_FORTIFY_SOURCE=y, runtime
> checks are always emitted where the compiler cannot guarantee a memcpy() is
> safely bounded, and compile-time warnings are enabled for W=1 builds.
>
> This catches a large class of common buffer overflow flaws, and would have
> caught all of the memcpy()-related buffer write overflow flaws identified in
> the last three years.
>
> As an aside, since W=1 is chock-full of (IMO useless) warnings, is there any
> way to enable *just* the FORTIFY_SOURCE warnings?
>
> [...]
>
> > Implementation:
> >
> > Tighten the memcpy() destination buffer size checking to use the actual
> > ("mode 1") target buffer size as the bounds check instead of their
> > enclosing structure's ("mode 0") size. Use a common inline for memcpy()
> > (and memmove() in a following patch), since all the tests are the
> > same. All new cross-field memcpy() uses must use the struct_group() macro
> > or similar to target a specific range of fields, so that FORTIFY_SOURCE
> > can reason about the size and safety of the copy.
> >
> > For now, cross-member "mode 1" read detection at compile-time will be
> > limited to W=1 builds, since it is, unfortunately, very common. As the
> > priority is solving write overflows, read overflows can be part of the
> > next phase.
>
> I had a go at testing this on arm64, and could get build-time warnings from GCC
> 11.1.0, but not from Clang 13.0.0.
Looking again, I see this is down to __compiletime_warning() only being usable
from clang 14.0 onwards (and the final patch mentions that in passing), so I
guess that's expected.
It would be nice to call that out somewhere in this patch (e.g. in that
introductory paragraph), since it's very each to miss that and get confused...
;)
Thanks,
Mark.
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-16 11:08 ` Mark Rutland
2021-12-16 11:21 ` Mark Rutland
@ 2021-12-16 18:00 ` Kees Cook
2021-12-17 13:34 ` Mark Rutland
1 sibling, 1 reply; 47+ messages in thread
From: Kees Cook @ 2021-12-16 18:00 UTC (permalink / raw)
To: Mark Rutland; +Cc: linux-hardening, linux-kernel
On Thu, Dec 16, 2021 at 11:08:26AM +0000, Mark Rutland wrote:
> On Mon, Dec 13, 2021 at 02:33:20PM -0800, Kees Cook wrote:
> > memcpy() is dead; long live memcpy()
> >
> > tl;dr: In order to eliminate a large class of common buffer overflow
> > flaws that continue to persist in the kernel, have memcpy() (under
> > CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct
> > member when they have a known size. This would have caught all of the
> > memcpy()-related buffer write overflow flaws identified in at least the
> > last three years.
> >
>
> Hi Kees,
>
> Since there's a *lot* of context below, it's very easy to miss some key details
> (e.g. that the compile-time warnings are limited to W=1 builds). It would be
> really nice if the summary above could say something like:
Hm, I do need to write a better summary! I think there's still some
misunderstanding, and I will attempt some clarity here... :)
>
> This patch makes it possible to detect when memcpy() of a struct member may
> go past the bounds of that member. When CONFIG_FORTIFY_SOURCE=y, runtime
> checks are always emitted where the compiler cannot guarantee a memcpy() is
> safely bounded, and compile-time warnings are enabled for W=1 builds.
For GCC and Clang 14, compile-time _write_ overflow warnings are meant
to be emitted under FORTIFY_SOURCE. _read_ overflow warnings are meant
to be emitted under FORTIFY_SOURCE + W=1 (or when the same statement
also has a write overflow).
>
> This catches a large class of common buffer overflow flaws, and would have
> caught all of the memcpy()-related buffer write overflow flaws identified in
> the last three years.
>
> As an aside, since W=1 is chock-full of (IMO useless) warnings, is there any
> way to enable *just* the FORTIFY_SOURCE warnings?
To see them all (i.e. not shove some into W=1), you can remove the "W=1
or write overflow" part of the read overflow test in fortify-string.h.
e.g.:
- if ((IS_ENABLED(KBUILD_EXTRA_WARN1) || p_size_field < size) &&
- q_size_field < size)
+ if (q_size_field < size)
> I had a go at testing this on arm64, and could get build-time warnings from GCC
> 11.1.0, but not from Clang 13.0.0.
This is correct and expected due to Clang 13's lack of support for
compiletime_warning().
> No relevant warnings, but code was generated for runtime warnings:
>
> | 0000000000000000 <foo_copy>:
> | 0: d503233f paciasp
> | 4: a9bf7bfd stp x29, x30, [sp, #-16]!
> | 8: 910003fd mov x29, sp
> | c: 52800080 mov w0, #0x4 // #4
> | 10: 52800101 mov w1, #0x8 // #8
> | 14: 94000000 bl 0 <__write_overflow_field>
> | 18: 52800080 mov w0, #0x4 // #4
> | 1c: 52800101 mov w1, #0x8 // #8
> | 20: 94000000 bl 0 <__read_overflow2_field>
> | 24: 90000008 adrp x8, 8 <foo_copy+0x8>
> | 28: f9400108 ldr x8, [x8]
> | 2c: 90000009 adrp x9, 0 <foo_copy>
> | 30: f9000128 str x8, [x9]
> | 34: a8c17bfd ldp x29, x30, [sp], #16
> | 38: d50323bf autiasp
> | 3c: d65f03c0 ret
>
> Have I misunderstood how that's meant to work, or am I doing something wrong?
The generally stated requirement from Linus for these kinds of
kernel changes was to never break the build (i.e. we cannot use
compiletime_error() -- which Clang 13 falls back to with a link-time
failure).
Since this phase of the series is only compile-time warnings (not the
run-time warnings), it's rather a no-op for Clang 13. However, the final
patch in the series brings the earlier ("mode 0") FORTIFY behaviors to
Clang finally.
Clang 14 implements compiletime_warning(), so in that situation, the
warnings appear.
It's a pretty wacky Venn Diagram, and I will attempt to include some
sort of illustration for it, as the behavioral differences are complex.
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time
2021-12-16 18:00 ` Kees Cook
@ 2021-12-17 13:34 ` Mark Rutland
0 siblings, 0 replies; 47+ messages in thread
From: Mark Rutland @ 2021-12-17 13:34 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-hardening, linux-kernel
On Thu, Dec 16, 2021 at 10:00:09AM -0800, Kees Cook wrote:
> On Thu, Dec 16, 2021 at 11:08:26AM +0000, Mark Rutland wrote:
> > On Mon, Dec 13, 2021 at 02:33:20PM -0800, Kees Cook wrote:
> > > memcpy() is dead; long live memcpy()
> > >
> > > tl;dr: In order to eliminate a large class of common buffer overflow
> > > flaws that continue to persist in the kernel, have memcpy() (under
> > > CONFIG_FORTIFY_SOURCE) perform bounds checking of the destination struct
> > > member when they have a known size. This would have caught all of the
> > > memcpy()-related buffer write overflow flaws identified in at least the
> > > last three years.
> > >
> >
> > Hi Kees,
> >
> > Since there's a *lot* of context below, it's very easy to miss some key details
> > (e.g. that the compile-time warnings are limited to W=1 builds). It would be
> > really nice if the summary above could say something like:
>
> Hm, I do need to write a better summary! I think there's still some
> misunderstanding, and I will attempt some clarity here... :)
Thanks! Sorry if that came across as a complaint; this looks really useful and
I just couldn't figure out if I was holding things wrong or whether I was
hitting an unexpected issue. :)
> > This patch makes it possible to detect when memcpy() of a struct member may
> > go past the bounds of that member. When CONFIG_FORTIFY_SOURCE=y, runtime
> > checks are always emitted where the compiler cannot guarantee a memcpy() is
> > safely bounded, and compile-time warnings are enabled for W=1 builds.
>
> For GCC and Clang 14, compile-time _write_ overflow warnings are meant
> to be emitted under FORTIFY_SOURCE. _read_ overflow warnings are meant
> to be emitted under FORTIFY_SOURCE + W=1 (or when the same statement
> also has a write overflow).
Cool.
I'll await a clang 14 release, or go build my own copy in the mean time.
> > This catches a large class of common buffer overflow flaws, and would have
> > caught all of the memcpy()-related buffer write overflow flaws identified in
> > the last three years.
> >
> > As an aside, since W=1 is chock-full of (IMO useless) warnings, is there any
> > way to enable *just* the FORTIFY_SOURCE warnings?
>
> To see them all (i.e. not shove some into W=1), you can remove the "W=1
> or write overflow" part of the read overflow test in fortify-string.h.
> e.g.:
>
> - if ((IS_ENABLED(KBUILD_EXTRA_WARN1) || p_size_field < size) &&
> - q_size_field < size)
> + if (q_size_field < size)
>
> > I had a go at testing this on arm64, and could get build-time warnings from GCC
> > 11.1.0, but not from Clang 13.0.0.
>
> This is correct and expected due to Clang 13's lack of support for
> compiletime_warning().
Thanks for confirming!
Thanks,
Mark.
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 07/17] fortify: Detect struct member overflows in memmove() at compile-time
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (5 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 06/17] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` Kees Cook
` (11 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, linux-kernel
As done for memcpy(), also update memmove() to use the same tightened
compile-time checks under CONFIG_FORTIFY_SOURCE.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/boot/compressed/misc.c | 3 ++-
arch/x86/lib/memcpy_32.c | 1 +
include/linux/fortify-string.h | 21 ++++---------------
.../read_overflow2_field-memmove.c | 5 +++++
.../write_overflow_field-memmove.c | 5 +++++
5 files changed, 17 insertions(+), 18 deletions(-)
create mode 100644 lib/test_fortify/read_overflow2_field-memmove.c
create mode 100644 lib/test_fortify/write_overflow_field-memmove.c
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a4339cb2d247..1cdcaf34ee36 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -37,10 +37,11 @@
* try to define their own functions if these are not defined as macros.
*/
#define memzero(s, n) memset((s), 0, (n))
+#ifndef memmove
#define memmove memmove
-
/* Functions used by the included decompressor code below. */
void *memmove(void *dest, const void *src, size_t n);
+#endif
/*
* This is set up by the setup-routine at boot-time
diff --git a/arch/x86/lib/memcpy_32.c b/arch/x86/lib/memcpy_32.c
index e565d1c9019e..f19b7fd07f04 100644
--- a/arch/x86/lib/memcpy_32.c
+++ b/arch/x86/lib/memcpy_32.c
@@ -4,6 +4,7 @@
#undef memcpy
#undef memset
+#undef memmove
__visible void *memcpy(void *to, const void *from, size_t n)
{
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 132efa1ff49f..c07871a3fcd0 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -309,22 +309,10 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
__builtin_object_size(p, 0), __builtin_object_size(q, 0), \
__builtin_object_size(p, 1), __builtin_object_size(q, 1), \
memcpy)
-
-__FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
-{
- size_t p_size = __builtin_object_size(p, 0);
- size_t q_size = __builtin_object_size(q, 0);
-
- if (__builtin_constant_p(size)) {
- if (p_size < size)
- __write_overflow();
- if (q_size < size)
- __read_overflow2();
- }
- if (p_size < size || q_size < size)
- fortify_panic(__func__);
- return __underlying_memmove(p, q, size);
-}
+#define memmove(p, q, s) __fortify_memcpy_chk(p, q, s, \
+ __builtin_object_size(p, 0), __builtin_object_size(q, 0), \
+ __builtin_object_size(p, 1), __builtin_object_size(q, 1), \
+ memmove)
extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
__FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
@@ -413,7 +401,6 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
/* Don't use these outside the FORITFY_SOURCE implementation */
#undef __underlying_memchr
#undef __underlying_memcmp
-#undef __underlying_memmove
#undef __underlying_memset
#undef __underlying_strcat
#undef __underlying_strcpy
diff --git a/lib/test_fortify/read_overflow2_field-memmove.c b/lib/test_fortify/read_overflow2_field-memmove.c
new file mode 100644
index 000000000000..6cc2724c8f62
--- /dev/null
+++ b/lib/test_fortify/read_overflow2_field-memmove.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST \
+ memmove(large, instance.buf, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
diff --git a/lib/test_fortify/write_overflow_field-memmove.c b/lib/test_fortify/write_overflow_field-memmove.c
new file mode 100644
index 000000000000..377fcf9bb2fd
--- /dev/null
+++ b/lib/test_fortify/write_overflow_field-memmove.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST \
+ memmove(instance.buf, large, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
` (17 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Kalle Valo, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_startat() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct. Additionally split up a later
field-spanning memset() so that memset() can reason about the size.
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/ath/ath11k/hal_rx.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/hal_rx.c b/drivers/net/wireless/ath/ath11k/hal_rx.c
index 329c404cfa80..0e43e215c10a 100644
--- a/drivers/net/wireless/ath/ath11k/hal_rx.c
+++ b/drivers/net/wireless/ath/ath11k/hal_rx.c
@@ -29,8 +29,7 @@ static int ath11k_hal_reo_cmd_queue_stats(struct hal_tlv_hdr *tlv,
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_get_queue_stats *)tlv->value;
- memset(&desc->queue_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, queue_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -62,8 +61,7 @@ static int ath11k_hal_reo_cmd_flush_cache(struct ath11k_hal *hal, struct hal_tlv
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_flush_cache *)tlv->value;
- memset(&desc->cache_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, cache_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -101,8 +99,7 @@ static int ath11k_hal_reo_cmd_update_rx_queue(struct hal_tlv_hdr *tlv,
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_update_rx_queue *)tlv->value;
- memset(&desc->queue_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, queue_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -764,15 +761,17 @@ void ath11k_hal_reo_qdesc_setup(void *vaddr, int tid, u32 ba_window_size,
* size changes and also send WMI message to FW to change the REO
* queue descriptor in Rx peer entry as part of dp_rx_tid_update.
*/
- memset(ext_desc, 0, 3 * sizeof(*ext_desc));
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_1);
ext_desc++;
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_2);
ext_desc++;
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_3);
--
2.30.2
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
@ 2021-12-13 22:33 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Kalle Valo, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_startat() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct. Additionally split up a later
field-spanning memset() so that memset() can reason about the size.
Cc: Kalle Valo <kvalo@codeaurora.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: ath11k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/ath/ath11k/hal_rx.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/hal_rx.c b/drivers/net/wireless/ath/ath11k/hal_rx.c
index 329c404cfa80..0e43e215c10a 100644
--- a/drivers/net/wireless/ath/ath11k/hal_rx.c
+++ b/drivers/net/wireless/ath/ath11k/hal_rx.c
@@ -29,8 +29,7 @@ static int ath11k_hal_reo_cmd_queue_stats(struct hal_tlv_hdr *tlv,
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_get_queue_stats *)tlv->value;
- memset(&desc->queue_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, queue_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -62,8 +61,7 @@ static int ath11k_hal_reo_cmd_flush_cache(struct ath11k_hal *hal, struct hal_tlv
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_flush_cache *)tlv->value;
- memset(&desc->cache_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, cache_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -101,8 +99,7 @@ static int ath11k_hal_reo_cmd_update_rx_queue(struct hal_tlv_hdr *tlv,
FIELD_PREP(HAL_TLV_HDR_LEN, sizeof(*desc));
desc = (struct hal_reo_update_rx_queue *)tlv->value;
- memset(&desc->queue_addr_lo, 0,
- (sizeof(*desc) - sizeof(struct hal_reo_cmd_hdr)));
+ memset_startat(desc, 0, queue_addr_lo);
desc->cmd.info0 &= ~HAL_REO_CMD_HDR_INFO0_STATUS_REQUIRED;
if (cmd->flag & HAL_REO_CMD_FLG_NEED_STATUS)
@@ -764,15 +761,17 @@ void ath11k_hal_reo_qdesc_setup(void *vaddr, int tid, u32 ba_window_size,
* size changes and also send WMI message to FW to change the REO
* queue descriptor in Rx peer entry as part of dp_rx_tid_update.
*/
- memset(ext_desc, 0, 3 * sizeof(*ext_desc));
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_1);
ext_desc++;
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_2);
ext_desc++;
+ memset(ext_desc, 0, sizeof(*ext_desc));
ath11k_hal_reo_set_desc_hdr(&ext_desc->desc_hdr, HAL_DESC_REO_OWNED,
HAL_DESC_REO_QUEUE_EXT_DESC,
REO_QUEUE_DESC_MAGIC_DEBUG_PATTERN_3);
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
2021-12-13 22:33 ` Kees Cook
@ 2021-12-14 6:02 ` Kalle Valo
-1 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-14 6:02 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kees Cook <keescook@chromium.org> writes:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
>
> Use memset_startat() so memset() doesn't get confused about writing
> beyond the destination member that is intended to be the starting point
> of zeroing through the end of the struct. Additionally split up a later
> field-spanning memset() so that memset() can reason about the size.
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: ath11k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
What's the plan for this patch? I would like to take this via my ath
tree to avoid conflicts.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
@ 2021-12-14 6:02 ` Kalle Valo
0 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-14 6:02 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kees Cook <keescook@chromium.org> writes:
> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> field bounds checking for memset(), avoid intentionally writing across
> neighboring fields.
>
> Use memset_startat() so memset() doesn't get confused about writing
> beyond the destination member that is intended to be the starting point
> of zeroing through the end of the struct. Additionally split up a later
> field-spanning memset() so that memset() can reason about the size.
>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: ath11k@lists.infradead.org
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
What's the plan for this patch? I would like to take this via my ath
tree to avoid conflicts.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
2021-12-14 6:02 ` Kalle Valo
@ 2021-12-14 15:46 ` Kalle Valo
-1 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-14 15:46 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kalle Valo <kvalo@kernel.org> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> field bounds checking for memset(), avoid intentionally writing across
>> neighboring fields.
>>
>> Use memset_startat() so memset() doesn't get confused about writing
>> beyond the destination member that is intended to be the starting point
>> of zeroing through the end of the struct. Additionally split up a later
>> field-spanning memset() so that memset() can reason about the size.
>>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: ath11k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> What's the plan for this patch? I would like to take this via my ath
> tree to avoid conflicts.
Actually this has been already applied:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
Why are you submitting the same patch twice?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
@ 2021-12-14 15:46 ` Kalle Valo
0 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-14 15:46 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kalle Valo <kvalo@kernel.org> writes:
> Kees Cook <keescook@chromium.org> writes:
>
>> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> field bounds checking for memset(), avoid intentionally writing across
>> neighboring fields.
>>
>> Use memset_startat() so memset() doesn't get confused about writing
>> beyond the destination member that is intended to be the starting point
>> of zeroing through the end of the struct. Additionally split up a later
>> field-spanning memset() so that memset() can reason about the size.
>>
>> Cc: Kalle Valo <kvalo@codeaurora.org>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: ath11k@lists.infradead.org
>> Cc: linux-wireless@vger.kernel.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>
> What's the plan for this patch? I would like to take this via my ath
> tree to avoid conflicts.
Actually this has been already applied:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
Why are you submitting the same patch twice?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
2021-12-14 15:46 ` Kalle Valo
@ 2021-12-14 17:05 ` Kees Cook
-1 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-14 17:05 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
On Tue, Dec 14, 2021 at 05:46:31PM +0200, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
>
> > Kees Cook <keescook@chromium.org> writes:
> >
> >> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> >> field bounds checking for memset(), avoid intentionally writing across
> >> neighboring fields.
> >>
> >> Use memset_startat() so memset() doesn't get confused about writing
> >> beyond the destination member that is intended to be the starting point
> >> of zeroing through the end of the struct. Additionally split up a later
> >> field-spanning memset() so that memset() can reason about the size.
> >>
> >> Cc: Kalle Valo <kvalo@codeaurora.org>
> >> Cc: "David S. Miller" <davem@davemloft.net>
> >> Cc: Jakub Kicinski <kuba@kernel.org>
> >> Cc: ath11k@lists.infradead.org
> >> Cc: linux-wireless@vger.kernel.org
> >> Cc: netdev@vger.kernel.org
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > What's the plan for this patch? I would like to take this via my ath
> > tree to avoid conflicts.
>
> Actually this has been already applied:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
>
> Why are you submitting the same patch twice?
These are all part of a topic branch, and the cover letter mentioned
that a set of them have already been taken but haven't appeared in -next
(which was delayed).
Sorry for the confusion!
--
Kees Cook
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
@ 2021-12-14 17:05 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-14 17:05 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
On Tue, Dec 14, 2021 at 05:46:31PM +0200, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
>
> > Kees Cook <keescook@chromium.org> writes:
> >
> >> In preparation for FORTIFY_SOURCE performing compile-time and run-time
> >> field bounds checking for memset(), avoid intentionally writing across
> >> neighboring fields.
> >>
> >> Use memset_startat() so memset() doesn't get confused about writing
> >> beyond the destination member that is intended to be the starting point
> >> of zeroing through the end of the struct. Additionally split up a later
> >> field-spanning memset() so that memset() can reason about the size.
> >>
> >> Cc: Kalle Valo <kvalo@codeaurora.org>
> >> Cc: "David S. Miller" <davem@davemloft.net>
> >> Cc: Jakub Kicinski <kuba@kernel.org>
> >> Cc: ath11k@lists.infradead.org
> >> Cc: linux-wireless@vger.kernel.org
> >> Cc: netdev@vger.kernel.org
> >> Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > What's the plan for this patch? I would like to take this via my ath
> > tree to avoid conflicts.
>
> Actually this has been already applied:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
>
> Why are you submitting the same patch twice?
These are all part of a topic branch, and the cover letter mentioned
that a set of them have already been taken but haven't appeared in -next
(which was delayed).
Sorry for the confusion!
--
Kees Cook
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
2021-12-14 17:05 ` Kees Cook
@ 2021-12-16 13:50 ` Kalle Valo
-1 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-16 13:50 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kees Cook <keescook@chromium.org> writes:
> On Tue, Dec 14, 2021 at 05:46:31PM +0200, Kalle Valo wrote:
>> Kalle Valo <kvalo@kernel.org> writes:
>>
>> > Kees Cook <keescook@chromium.org> writes:
>> >
>> >> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> >> field bounds checking for memset(), avoid intentionally writing across
>> >> neighboring fields.
>> >>
>> >> Use memset_startat() so memset() doesn't get confused about writing
>> >> beyond the destination member that is intended to be the starting point
>> >> of zeroing through the end of the struct. Additionally split up a later
>> >> field-spanning memset() so that memset() can reason about the size.
>> >>
>> >> Cc: Kalle Valo <kvalo@codeaurora.org>
>> >> Cc: "David S. Miller" <davem@davemloft.net>
>> >> Cc: Jakub Kicinski <kuba@kernel.org>
>> >> Cc: ath11k@lists.infradead.org
>> >> Cc: linux-wireless@vger.kernel.org
>> >> Cc: netdev@vger.kernel.org
>> >> Signed-off-by: Kees Cook <keescook@chromium.org>
>> >
>> > What's the plan for this patch? I would like to take this via my ath
>> > tree to avoid conflicts.
>>
>> Actually this has been already applied:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
>>
>> Why are you submitting the same patch twice?
>
> These are all part of a topic branch, and the cover letter mentioned
> that a set of them have already been taken but haven't appeared in -next
> (which was delayed).
Do note that some wireless drivers (at least ath, mt76 and iwlwifi) are
maintained in separate trees, so don't be surprised if it takes several
weeks before they are visible in linux-next.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 08/17] ath11k: Use memset_startat() for clearing queue descriptors
@ 2021-12-16 13:50 ` Kalle Valo
0 siblings, 0 replies; 47+ messages in thread
From: Kalle Valo @ 2021-12-16 13:50 UTC (permalink / raw)
To: Kees Cook
Cc: linux-hardening, David S. Miller, Jakub Kicinski, ath11k,
linux-wireless, netdev, linux-kernel
Kees Cook <keescook@chromium.org> writes:
> On Tue, Dec 14, 2021 at 05:46:31PM +0200, Kalle Valo wrote:
>> Kalle Valo <kvalo@kernel.org> writes:
>>
>> > Kees Cook <keescook@chromium.org> writes:
>> >
>> >> In preparation for FORTIFY_SOURCE performing compile-time and run-time
>> >> field bounds checking for memset(), avoid intentionally writing across
>> >> neighboring fields.
>> >>
>> >> Use memset_startat() so memset() doesn't get confused about writing
>> >> beyond the destination member that is intended to be the starting point
>> >> of zeroing through the end of the struct. Additionally split up a later
>> >> field-spanning memset() so that memset() can reason about the size.
>> >>
>> >> Cc: Kalle Valo <kvalo@codeaurora.org>
>> >> Cc: "David S. Miller" <davem@davemloft.net>
>> >> Cc: Jakub Kicinski <kuba@kernel.org>
>> >> Cc: ath11k@lists.infradead.org
>> >> Cc: linux-wireless@vger.kernel.org
>> >> Cc: netdev@vger.kernel.org
>> >> Signed-off-by: Kees Cook <keescook@chromium.org>
>> >
>> > What's the plan for this patch? I would like to take this via my ath
>> > tree to avoid conflicts.
>>
>> Actually this has been already applied:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=d5549e9a6b86
>>
>> Why are you submitting the same patch twice?
>
> These are all part of a topic branch, and the cover letter mentioned
> that a set of them have already been taken but haven't appeared in -next
> (which was delayed).
Do note that some wireless drivers (at least ath, mt76 and iwlwifi) are
maintained in separate trees, so don't be surprised if it takes several
weeks before they are visible in linux-next.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 09/17] RDMA/mlx5: Use memset_after() to zero struct mlx5_ib_mr
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (7 preceding siblings ...)
2021-12-13 22:33 ` Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [Drbd-dev] " Kees Cook
` (9 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Doug Ledford, Jason Gunthorpe, linux-rdma,
Leon Romanovsky, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_after() to zero the end of struct mlx5_ib_mr that should
be initialized.
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org
Acked-by: Leon Romanovsky <leonro@nvidia.com>
Link: https://lore.kernel.org/lkml/YbByJSkBgLRp5S8V@unreal
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 4a7a56ed740b..ded10719b643 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -664,8 +664,8 @@ struct mlx5_ib_mr {
/* User MR data */
struct mlx5_cache_ent *cache_ent;
+ /* Everything after cache_ent is zero'd when MR allocated */
- /* This is zero'd when the MR is allocated */
union {
/* Used only while the MR is in the cache */
struct {
@@ -718,7 +718,7 @@ struct mlx5_ib_mr {
/* Zero the fields in the mr that are variant depending on usage */
static inline void mlx5_clear_mr(struct mlx5_ib_mr *mr)
{
- memset(mr->out, 0, sizeof(*mr) - offsetof(struct mlx5_ib_mr, out));
+ memset_after(mr, 0, cache_ent);
}
static inline bool is_odp_mr(struct mlx5_ib_mr *mr)
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 10/17] drbd: Use struct_group() to zero algs
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
` (17 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Philipp Reisner, Lars Ellenberg, Jens Axboe, drbd-dev,
linux-block, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Add a struct_group() for the algs so that memset() can correctly reason
about the size. "objdump -d" shows no changes to the executable
instructions.
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: drbd-dev@lists.linbit.com
Cc: linux-block@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/block/drbd/drbd_main.c | 3 ++-
drivers/block/drbd/drbd_protocol.h | 6 ++++--
drivers/block/drbd/drbd_receiver.c | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 07b3c6093e7d..6f450816c4fa 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -729,7 +729,8 @@ int drbd_send_sync_param(struct drbd_peer_device *peer_device)
cmd = apv >= 89 ? P_SYNC_PARAM89 : P_SYNC_PARAM;
/* initialize verify_alg and csums_alg */
- memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+ BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+ memset(&p->algs, 0, sizeof(p->algs));
if (get_ldev(peer_device->device)) {
dc = rcu_dereference(peer_device->device->ldev->disk_conf);
diff --git a/drivers/block/drbd/drbd_protocol.h b/drivers/block/drbd/drbd_protocol.h
index dea59c92ecc1..a882b65ab5d2 100644
--- a/drivers/block/drbd/drbd_protocol.h
+++ b/drivers/block/drbd/drbd_protocol.h
@@ -283,8 +283,10 @@ struct p_rs_param_89 {
struct p_rs_param_95 {
u32 resync_rate;
- char verify_alg[SHARED_SECRET_MAX];
- char csums_alg[SHARED_SECRET_MAX];
+ struct_group(algs,
+ char verify_alg[SHARED_SECRET_MAX];
+ char csums_alg[SHARED_SECRET_MAX];
+ );
u32 c_plan_ahead;
u32 c_delay_target;
u32 c_fill_target;
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 1f740e42e457..6df2539e215b 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3921,7 +3921,8 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
/* initialize verify_alg and csums_alg */
p = pi->data;
- memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+ BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+ memset(&p->algs, 0, sizeof(p->algs));
err = drbd_recv_all(peer_device->connection, p, header_size);
if (err)
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [Drbd-dev] [PATCH 10/17] drbd: Use struct_group() to zero algs
@ 2021-12-13 22:33 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Jens Axboe, Kees Cook, Philipp Reisner, linux-kernel, linux-block,
Lars Ellenberg, drbd-dev
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Add a struct_group() for the algs so that memset() can correctly reason
about the size. "objdump -d" shows no changes to the executable
instructions.
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: drbd-dev@lists.linbit.com
Cc: linux-block@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/block/drbd/drbd_main.c | 3 ++-
drivers/block/drbd/drbd_protocol.h | 6 ++++--
drivers/block/drbd/drbd_receiver.c | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 07b3c6093e7d..6f450816c4fa 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -729,7 +729,8 @@ int drbd_send_sync_param(struct drbd_peer_device *peer_device)
cmd = apv >= 89 ? P_SYNC_PARAM89 : P_SYNC_PARAM;
/* initialize verify_alg and csums_alg */
- memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+ BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+ memset(&p->algs, 0, sizeof(p->algs));
if (get_ldev(peer_device->device)) {
dc = rcu_dereference(peer_device->device->ldev->disk_conf);
diff --git a/drivers/block/drbd/drbd_protocol.h b/drivers/block/drbd/drbd_protocol.h
index dea59c92ecc1..a882b65ab5d2 100644
--- a/drivers/block/drbd/drbd_protocol.h
+++ b/drivers/block/drbd/drbd_protocol.h
@@ -283,8 +283,10 @@ struct p_rs_param_89 {
struct p_rs_param_95 {
u32 resync_rate;
- char verify_alg[SHARED_SECRET_MAX];
- char csums_alg[SHARED_SECRET_MAX];
+ struct_group(algs,
+ char verify_alg[SHARED_SECRET_MAX];
+ char csums_alg[SHARED_SECRET_MAX];
+ );
u32 c_plan_ahead;
u32 c_delay_target;
u32 c_fill_target;
diff --git a/drivers/block/drbd/drbd_receiver.c b/drivers/block/drbd/drbd_receiver.c
index 1f740e42e457..6df2539e215b 100644
--- a/drivers/block/drbd/drbd_receiver.c
+++ b/drivers/block/drbd/drbd_receiver.c
@@ -3921,7 +3921,8 @@ static int receive_SyncParam(struct drbd_connection *connection, struct packet_i
/* initialize verify_alg and csums_alg */
p = pi->data;
- memset(p->verify_alg, 0, 2 * SHARED_SECRET_MAX);
+ BUILD_BUG_ON(sizeof(p->algs) != 2 * SHARED_SECRET_MAX);
+ memset(&p->algs, 0, sizeof(p->algs));
err = drbd_recv_all(peer_device->connection, p, header_size);
if (err)
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [dm-devel] [PATCH 11/17] dm integrity: Use struct_group() to zero struct journal_sector
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 02/17] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
` (17 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Mike Snitzer, dm-devel, Kees Cook, Alasdair Kergon, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Add struct_group() to mark region of struct journal_sector that should be
initialized to zero.
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: dm-devel@redhat.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/md/dm-integrity.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 6319deccbe09..163c94ca4e5c 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -121,8 +121,10 @@ struct journal_entry {
#define JOURNAL_MAC_SIZE (JOURNAL_MAC_PER_SECTOR * JOURNAL_BLOCK_SECTORS)
struct journal_sector {
- __u8 entries[JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR];
- __u8 mac[JOURNAL_MAC_PER_SECTOR];
+ struct_group(sectors,
+ __u8 entries[JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR];
+ __u8 mac[JOURNAL_MAC_PER_SECTOR];
+ );
commit_id_t commit_id;
};
@@ -2870,7 +2872,8 @@ static void init_journal(struct dm_integrity_c *ic, unsigned start_section,
wraparound_section(ic, &i);
for (j = 0; j < ic->journal_section_sectors; j++) {
struct journal_sector *js = access_journal(ic, i, j);
- memset(&js->entries, 0, JOURNAL_SECTOR_DATA);
+ BUILD_BUG_ON(sizeof(js->sectors) != JOURNAL_SECTOR_DATA);
+ memset(&js->sectors, 0, sizeof(js->sectors));
js->commit_id = dm_integrity_commit_id(ic, i, j, commit_seq);
}
for (j = 0; j < ic->journal_section_entries; j++) {
--
2.30.2
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 11/17] dm integrity: Use struct_group() to zero struct journal_sector
@ 2021-12-13 22:33 ` Kees Cook
0 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Alasdair Kergon, Mike Snitzer, dm-devel, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Add struct_group() to mark region of struct journal_sector that should be
initialized to zero.
Cc: Alasdair Kergon <agk@redhat.com>
Cc: Mike Snitzer <snitzer@redhat.com>
Cc: dm-devel@redhat.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/md/dm-integrity.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 6319deccbe09..163c94ca4e5c 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -121,8 +121,10 @@ struct journal_entry {
#define JOURNAL_MAC_SIZE (JOURNAL_MAC_PER_SECTOR * JOURNAL_BLOCK_SECTORS)
struct journal_sector {
- __u8 entries[JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR];
- __u8 mac[JOURNAL_MAC_PER_SECTOR];
+ struct_group(sectors,
+ __u8 entries[JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR];
+ __u8 mac[JOURNAL_MAC_PER_SECTOR];
+ );
commit_id_t commit_id;
};
@@ -2870,7 +2872,8 @@ static void init_journal(struct dm_integrity_c *ic, unsigned start_section,
wraparound_section(ic, &i);
for (j = 0; j < ic->journal_section_sectors; j++) {
struct journal_sector *js = access_journal(ic, i, j);
- memset(&js->entries, 0, JOURNAL_SECTOR_DATA);
+ BUILD_BUG_ON(sizeof(js->sectors) != JOURNAL_SECTOR_DATA);
+ memset(&js->sectors, 0, sizeof(js->sectors));
js->commit_id = dm_integrity_commit_id(ic, i, j, commit_seq);
}
for (j = 0; j < ic->journal_section_entries; j++) {
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 12/17] iw_cxgb4: Use memset_startat() for cpl_t5_pass_accept_rpl
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (10 preceding siblings ...)
2021-12-13 22:33 ` Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 13/17] intel_th: msu: Use memset_startat() for clearing hw header Kees Cook
` (6 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Potnuri Bharat Teja, Doug Ledford, Jason Gunthorpe,
Raju Rangoju, David S. Miller, Jakub Kicinski, linux-rdma, netdev,
linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_startat() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct. Additionally, since everything
appears to perform a roundup (including allocation), just change the
size of the struct itself and add a build-time check to validate the
expected size.
Cc: Potnuri Bharat Teja <bharat@chelsio.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Raju Rangoju <rajur@chelsio.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-rdma@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/infiniband/hw/cxgb4/cm.c | 5 +++--
drivers/net/ethernet/chelsio/cxgb4/t4_msg.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/cxgb4/cm.c b/drivers/infiniband/hw/cxgb4/cm.c
index 913f39ee4416..c16017f6e8db 100644
--- a/drivers/infiniband/hw/cxgb4/cm.c
+++ b/drivers/infiniband/hw/cxgb4/cm.c
@@ -2471,7 +2471,8 @@ static int accept_cr(struct c4iw_ep *ep, struct sk_buff *skb,
skb_get(skb);
rpl = cplhdr(skb);
if (!is_t4(adapter_type)) {
- skb_trim(skb, roundup(sizeof(*rpl5), 16));
+ BUILD_BUG_ON(sizeof(*rpl5) != roundup(sizeof(*rpl5), 16));
+ skb_trim(skb, sizeof(*rpl5));
rpl5 = (void *)rpl;
INIT_TP_WR(rpl5, ep->hwtid);
} else {
@@ -2487,7 +2488,7 @@ static int accept_cr(struct c4iw_ep *ep, struct sk_buff *skb,
opt2 |= CONG_CNTRL_V(CONG_ALG_TAHOE);
opt2 |= T5_ISS_F;
rpl5 = (void *)rpl;
- memset(&rpl5->iss, 0, roundup(sizeof(*rpl5)-sizeof(*rpl), 16));
+ memset_after(rpl5, 0, iss);
if (peer2peer)
isn += 4;
rpl5->iss = cpu_to_be32(isn);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_msg.h b/drivers/net/ethernet/chelsio/cxgb4/t4_msg.h
index fed5f93bf620..26433a62d7f0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_msg.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_msg.h
@@ -497,7 +497,7 @@ struct cpl_t5_pass_accept_rpl {
__be32 opt2;
__be64 opt0;
__be32 iss;
- __be32 rsvd;
+ __be32 rsvd[3];
};
struct cpl_act_open_req {
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 13/17] intel_th: msu: Use memset_startat() for clearing hw header
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (11 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 12/17] iw_cxgb4: Use memset_startat() for cpl_t5_pass_accept_rpl Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 14/17] IB/mthca: Use memset_startat() for clearing mpt_entry Kees Cook
` (5 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, Alexander Shishkin, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_startat() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct.
Acked-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Link: https://lore.kernel.org/lkml/87sfyzi97l.fsf@ashishki-desk.ger.corp.intel.com
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/hwtracing/intel_th/msu.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 432ade0842f6..70a07b4e9967 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -658,13 +658,11 @@ static void msc_buffer_clear_hw_header(struct msc *msc)
list_for_each_entry(win, &msc->win_list, entry) {
unsigned int blk;
- size_t hw_sz = sizeof(struct msc_block_desc) -
- offsetof(struct msc_block_desc, hw_tag);
for_each_sg(win->sgt->sgl, sg, win->nr_segs, blk) {
struct msc_block_desc *bdesc = sg_virt(sg);
- memset(&bdesc->hw_tag, 0, hw_sz);
+ memset_startat(bdesc, 0, hw_tag);
}
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 14/17] IB/mthca: Use memset_startat() for clearing mpt_entry
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (12 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 13/17] intel_th: msu: Use memset_startat() for clearing hw header Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 15/17] scsi: lpfc: Use struct_group() to initialize struct lpfc_cgn_info Kees Cook
` (4 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, Doug Ledford, Jason Gunthorpe, Max Gurtovoy,
linux-rdma, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Use memset_startat() so memset() doesn't get confused about writing
beyond the destination member that is intended to be the starting point
of zeroing through the end of the struct.
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Max Gurtovoy <maxg@mellanox.com>
Cc: linux-rdma@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/infiniband/hw/mthca/mthca_mr.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/mthca/mthca_mr.c b/drivers/infiniband/hw/mthca/mthca_mr.c
index a59100c496b4..192f83fd7c8a 100644
--- a/drivers/infiniband/hw/mthca/mthca_mr.c
+++ b/drivers/infiniband/hw/mthca/mthca_mr.c
@@ -467,8 +467,7 @@ int mthca_mr_alloc(struct mthca_dev *dev, u32 pd, int buffer_size_shift,
mpt_entry->start = cpu_to_be64(iova);
mpt_entry->length = cpu_to_be64(total_size);
- memset(&mpt_entry->lkey, 0,
- sizeof *mpt_entry - offsetof(struct mthca_mpt_entry, lkey));
+ memset_startat(mpt_entry, 0, lkey);
if (mr->mtt)
mpt_entry->mtt_seg =
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 15/17] scsi: lpfc: Use struct_group() to initialize struct lpfc_cgn_info
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (13 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 14/17] IB/mthca: Use memset_startat() for clearing mpt_entry Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-13 22:33 ` [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
` (3 subsequent siblings)
18 siblings, 0 replies; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening
Cc: Kees Cook, James Smart, Dick Kennedy, James E.J. Bottomley,
Martin K. Petersen, linux-scsi, linux-kernel
In preparation for FORTIFY_SOURCE performing compile-time and run-time
field bounds checking for memset(), avoid intentionally writing across
neighboring fields.
Add struct_group() to mark "stat" region of struct lpfc_cgn_info that
should be initialized to zero, and refactor the "data" region memset()
to wipe everything up to the cgn_stats region.
Cc: James Smart <james.smart@broadcom.com>
Cc: Dick Kennedy <dick.kennedy@broadcom.com>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/scsi/lpfc/lpfc.h | 90 +++++++++++++++++------------------
drivers/scsi/lpfc/lpfc_init.c | 4 +-
2 files changed, 46 insertions(+), 48 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc.h b/drivers/scsi/lpfc/lpfc.h
index 3faadcfcdcbb..4878c94761f9 100644
--- a/drivers/scsi/lpfc/lpfc.h
+++ b/drivers/scsi/lpfc/lpfc.h
@@ -496,52 +496,50 @@ struct lpfc_cgn_info {
__le32 cgn_alarm_hr[24];
__le32 cgn_alarm_day[LPFC_MAX_CGN_DAYS];
- /* Start of congestion statistics */
- uint8_t cgn_stat_npm; /* Notifications per minute */
-
- /* Start Time */
- uint8_t cgn_stat_month;
- uint8_t cgn_stat_day;
- uint8_t cgn_stat_year;
- uint8_t cgn_stat_hour;
- uint8_t cgn_stat_minute;
- uint8_t cgn_pad2[2];
-
- __le32 cgn_notification;
- __le32 cgn_peer_notification;
- __le32 link_integ_notification;
- __le32 delivery_notification;
-
- uint8_t cgn_stat_cgn_month; /* Last congestion notification FPIN */
- uint8_t cgn_stat_cgn_day;
- uint8_t cgn_stat_cgn_year;
- uint8_t cgn_stat_cgn_hour;
- uint8_t cgn_stat_cgn_min;
- uint8_t cgn_stat_cgn_sec;
-
- uint8_t cgn_stat_peer_month; /* Last peer congestion FPIN */
- uint8_t cgn_stat_peer_day;
- uint8_t cgn_stat_peer_year;
- uint8_t cgn_stat_peer_hour;
- uint8_t cgn_stat_peer_min;
- uint8_t cgn_stat_peer_sec;
-
- uint8_t cgn_stat_lnk_month; /* Last link integrity FPIN */
- uint8_t cgn_stat_lnk_day;
- uint8_t cgn_stat_lnk_year;
- uint8_t cgn_stat_lnk_hour;
- uint8_t cgn_stat_lnk_min;
- uint8_t cgn_stat_lnk_sec;
-
- uint8_t cgn_stat_del_month; /* Last delivery notification FPIN */
- uint8_t cgn_stat_del_day;
- uint8_t cgn_stat_del_year;
- uint8_t cgn_stat_del_hour;
- uint8_t cgn_stat_del_min;
- uint8_t cgn_stat_del_sec;
-#define LPFC_CGN_STAT_SIZE 48
-#define LPFC_CGN_DATA_SIZE (sizeof(struct lpfc_cgn_info) - \
- LPFC_CGN_STAT_SIZE - sizeof(uint32_t))
+ struct_group(cgn_stat,
+ uint8_t cgn_stat_npm; /* Notifications per minute */
+
+ /* Start Time */
+ uint8_t cgn_stat_month;
+ uint8_t cgn_stat_day;
+ uint8_t cgn_stat_year;
+ uint8_t cgn_stat_hour;
+ uint8_t cgn_stat_minute;
+ uint8_t cgn_pad2[2];
+
+ __le32 cgn_notification;
+ __le32 cgn_peer_notification;
+ __le32 link_integ_notification;
+ __le32 delivery_notification;
+
+ uint8_t cgn_stat_cgn_month; /* Last congestion notification FPIN */
+ uint8_t cgn_stat_cgn_day;
+ uint8_t cgn_stat_cgn_year;
+ uint8_t cgn_stat_cgn_hour;
+ uint8_t cgn_stat_cgn_min;
+ uint8_t cgn_stat_cgn_sec;
+
+ uint8_t cgn_stat_peer_month; /* Last peer congestion FPIN */
+ uint8_t cgn_stat_peer_day;
+ uint8_t cgn_stat_peer_year;
+ uint8_t cgn_stat_peer_hour;
+ uint8_t cgn_stat_peer_min;
+ uint8_t cgn_stat_peer_sec;
+
+ uint8_t cgn_stat_lnk_month; /* Last link integrity FPIN */
+ uint8_t cgn_stat_lnk_day;
+ uint8_t cgn_stat_lnk_year;
+ uint8_t cgn_stat_lnk_hour;
+ uint8_t cgn_stat_lnk_min;
+ uint8_t cgn_stat_lnk_sec;
+
+ uint8_t cgn_stat_del_month; /* Last delivery notification FPIN */
+ uint8_t cgn_stat_del_day;
+ uint8_t cgn_stat_del_year;
+ uint8_t cgn_stat_del_hour;
+ uint8_t cgn_stat_del_min;
+ uint8_t cgn_stat_del_sec;
+ );
__le32 cgn_info_crc;
#define LPFC_CGN_CRC32_MAGIC_NUMBER 0x1EDC6F41
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 2fe7d9d885d9..c18000d05379 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -13483,7 +13483,7 @@ lpfc_init_congestion_buf(struct lpfc_hba *phba)
phba->cgn_evt_minute = 0;
phba->hba_flag &= ~HBA_CGN_DAY_WRAP;
- memset(cp, 0xff, LPFC_CGN_DATA_SIZE);
+ memset(cp, 0xff, offsetof(struct lpfc_cgn_info, cgn_stat));
cp->cgn_info_size = cpu_to_le16(LPFC_CGN_INFO_SZ);
cp->cgn_info_version = LPFC_CGN_INFO_V3;
@@ -13542,7 +13542,7 @@ lpfc_init_congestion_stat(struct lpfc_hba *phba)
return;
cp = (struct lpfc_cgn_info *)phba->cgn_i->virt;
- memset(&cp->cgn_stat_npm, 0, LPFC_CGN_STAT_SIZE);
+ memset(&cp->cgn_stat, 0, sizeof(cp->cgn_stat));
ktime_get_real_ts64(&cmpl_time);
time64_to_tm(cmpl_time.tv_sec, 0, &broken);
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (14 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 15/17] scsi: lpfc: Use struct_group() to initialize struct lpfc_cgn_info Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-14 12:31 ` kernel test robot
2021-12-13 22:33 ` [PATCH 17/17] fortify: Work around Clang inlining bugs Kees Cook
` (2 subsequent siblings)
18 siblings, 1 reply; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, linux-kernel
As done for memcpy(), also update memset() to use the same tightened
compile-time bounds checking under CONFIG_FORTIFY_SOURCE.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 54 ++++++++++++++++---
.../write_overflow_field-memset.c | 5 ++
2 files changed, 51 insertions(+), 8 deletions(-)
create mode 100644 lib/test_fortify/write_overflow_field-memset.c
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c07871a3fcd0..c45159dbdaa1 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -200,17 +200,56 @@ __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
return p;
}
-__FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
+__FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
+ const size_t p_size,
+ const size_t p_size_field)
{
- size_t p_size = __builtin_object_size(p, 0);
+ if (__builtin_constant_p(size)) {
+ /*
+ * Length argument is a constant expression, so we
+ * can perform compile-time bounds checking where
+ * buffer sizes are known.
+ */
- if (__builtin_constant_p(size) && p_size < size)
- __write_overflow();
- if (p_size < size)
- fortify_panic(__func__);
- return __underlying_memset(p, c, size);
+ /* Error when size is larger than enclosing struct. */
+ if (p_size > p_size_field && p_size < size)
+ __write_overflow();
+
+ /* Warn when write size is larger than dest field. */
+ if (p_size_field < size)
+ __write_overflow_field(p_size_field, size);
+ }
+ /*
+ * At this point, length argument may not be a constant expression,
+ * so run-time bounds checking can be done where buffer sizes are
+ * known. (This is not an "else" because the above checks may only
+ * be compile-time warnings, and we want to still warn for run-time
+ * overflows.)
+ */
+
+ /*
+ * Always stop accesses beyond the struct that contains the
+ * field, when the buffer's remaining size is known.
+ * (The -1 test is to optimize away checks where the buffer
+ * lengths are unknown.)
+ */
+ if (p_size != (size_t)(-1) && p_size < size)
+ fortify_panic("memset");
}
+#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
+ size_t __fortify_size = (size_t)(size); \
+ fortify_memset_chk(__fortify_size, p_size, p_size_field), \
+ __underlying_memset(p, c, __fortify_size); \
+})
+
+/*
+ * __builtin_object_size() must be captured here to avoid evaluating argument
+ * side-effects further into the macro layers.
+ */
+#define memset(p, c, s) __fortify_memset_chk(p, c, s, \
+ __builtin_object_size(p, 0), __builtin_object_size(p, 1))
+
/*
* To make sure the compiler can enforce protection against buffer overflows,
* memcpy(), memmove(), and memset() must not be used beyond individual
@@ -401,7 +440,6 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
/* Don't use these outside the FORITFY_SOURCE implementation */
#undef __underlying_memchr
#undef __underlying_memcmp
-#undef __underlying_memset
#undef __underlying_strcat
#undef __underlying_strcpy
#undef __underlying_strlen
diff --git a/lib/test_fortify/write_overflow_field-memset.c b/lib/test_fortify/write_overflow_field-memset.c
new file mode 100644
index 000000000000..2331da26909e
--- /dev/null
+++ b/lib/test_fortify/write_overflow_field-memset.c
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define TEST \
+ memset(instance.buf, 0x42, sizeof(instance.buf) + 1)
+
+#include "test_fortify.h"
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time
2021-12-13 22:33 ` [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
@ 2021-12-14 12:31 ` kernel test robot
0 siblings, 0 replies; 47+ messages in thread
From: kernel test robot @ 2021-12-14 12:31 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 15624 bytes --]
Hi Kees,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.16-rc5]
[cannot apply to rdma/for-next axboe-block/for-next kvm/queue tip/x86/core mkp-scsi/for-next jejb-scsi/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git aa50faff4416c869b52dff68a937c84d29e12f4b
config: i386-randconfig-s001-20211214 (https://download.01.org/0day-ci/archive/20211214/202112142014.LCVuJteC-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
# https://github.com/0day-ci/linux/commit/5827537ce9a9fe9059d982a74a6cd7afeccc9145
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kees-Cook/Enable-strict-compile-time-memcpy-fortify-checks/20211214-064002
git checkout 5827537ce9a9fe9059d982a74a6cd7afeccc9145
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/video/fbdev/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/video/fbdev/i740fb.c:743:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got char [noderef] __iomem *screen_base @@
drivers/video/fbdev/i740fb.c:743:9: sparse: expected void const *
drivers/video/fbdev/i740fb.c:743:9: sparse: got char [noderef] __iomem *screen_base
>> drivers/video/fbdev/i740fb.c:743:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got char [noderef] __iomem *screen_base @@
drivers/video/fbdev/i740fb.c:743:9: sparse: expected void const *
drivers/video/fbdev/i740fb.c:743:9: sparse: got char [noderef] __iomem *screen_base
drivers/video/fbdev/i740fb.c:743:9: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void * @@ got char [noderef] __iomem *screen_base @@
drivers/video/fbdev/i740fb.c:743:9: sparse: expected void *
drivers/video/fbdev/i740fb.c:743:9: sparse: got char [noderef] __iomem *screen_base
vim +743 drivers/video/fbdev/i740fb.c
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 732
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 733 static int i740fb_set_par(struct fb_info *info)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 734 {
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 735 struct i740fb_par *par = info->par;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 736 u32 itemp;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 737 int i;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 738
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 739 i = i740fb_decode_var(&info->var, par, info);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 740 if (i)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 741 return i;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 742
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 @743 memset(info->screen_base, 0, info->screen_size);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 744
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 745 vga_protect(par);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 746
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 747 i740outreg(par, XRX, DRAM_EXT_CNTL, DRAM_REFRESH_DISABLE);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 748
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 749 mdelay(1);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 750
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 751 i740outreg(par, XRX, VCLK2_VCO_M, par->video_clk2_m);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 752 i740outreg(par, XRX, VCLK2_VCO_N, par->video_clk2_n);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 753 i740outreg(par, XRX, VCLK2_VCO_MN_MSBS, par->video_clk2_mn_msbs);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 754 i740outreg(par, XRX, VCLK2_VCO_DIV_SEL, par->video_clk2_div_sel);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 755
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 756 i740outreg_mask(par, XRX, PIXPIPE_CONFIG_0,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 757 par->pixelpipe_cfg0 & DAC_8_BIT, 0x80);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 758
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 759 i740inb(par, 0x3DA);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 760 i740outb(par, 0x3C0, 0x00);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 761
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 762 /* update misc output register */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 763 i740outb(par, VGA_MIS_W, par->misc | 0x01);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 764
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 765 /* synchronous reset on */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 766 i740outreg(par, VGA_SEQ_I, VGA_SEQ_RESET, 0x01);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 767 /* write sequencer registers */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 768 i740outreg(par, VGA_SEQ_I, VGA_SEQ_CLOCK_MODE,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 769 par->seq[VGA_SEQ_CLOCK_MODE] | 0x20);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 770 for (i = 2; i < VGA_SEQ_C; i++)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 771 i740outreg(par, VGA_SEQ_I, i, par->seq[i]);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 772
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 773 /* synchronous reset off */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 774 i740outreg(par, VGA_SEQ_I, VGA_SEQ_RESET, 0x03);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 775
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 776 /* deprotect CRT registers 0-7 */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 777 i740outreg(par, VGA_CRT_IC, VGA_CRTC_V_SYNC_END,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 778 par->crtc[VGA_CRTC_V_SYNC_END]);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 779
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 780 /* write CRT registers */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 781 for (i = 0; i < VGA_CRT_C; i++)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 782 i740outreg(par, VGA_CRT_IC, i, par->crtc[i]);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 783
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 784 /* write graphics controller registers */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 785 for (i = 0; i < VGA_GFX_C; i++)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 786 i740outreg(par, VGA_GFX_I, i, par->gdc[i]);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 787
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 788 /* write attribute controller registers */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 789 for (i = 0; i < VGA_ATT_C; i++) {
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 790 i740inb(par, VGA_IS1_RC); /* reset flip-flop */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 791 i740outb(par, VGA_ATT_IW, i);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 792 i740outb(par, VGA_ATT_IW, par->atc[i]);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 793 }
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 794
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 795 i740inb(par, VGA_IS1_RC);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 796 i740outb(par, VGA_ATT_IW, 0x20);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 797
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 798 i740outreg(par, VGA_CRT_IC, EXT_VERT_TOTAL, par->ext_vert_total);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 799 i740outreg(par, VGA_CRT_IC, EXT_VERT_DISPLAY, par->ext_vert_disp_end);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 800 i740outreg(par, VGA_CRT_IC, EXT_VERT_SYNC_START,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 801 par->ext_vert_sync_start);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 802 i740outreg(par, VGA_CRT_IC, EXT_VERT_BLANK_START,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 803 par->ext_vert_blank_start);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 804 i740outreg(par, VGA_CRT_IC, EXT_HORIZ_TOTAL, par->ext_horiz_total);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 805 i740outreg(par, VGA_CRT_IC, EXT_HORIZ_BLANK, par->ext_horiz_blank);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 806 i740outreg(par, VGA_CRT_IC, EXT_OFFSET, par->ext_offset);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 807 i740outreg(par, VGA_CRT_IC, EXT_START_ADDR_HI, par->ext_start_addr_hi);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 808 i740outreg(par, VGA_CRT_IC, EXT_START_ADDR, par->ext_start_addr);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 809
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 810 i740outreg_mask(par, VGA_CRT_IC, INTERLACE_CNTL,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 811 par->interlace_cntl, INTERLACE_ENABLE);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 812 i740outreg_mask(par, XRX, ADDRESS_MAPPING, par->address_mapping, 0x1F);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 813 i740outreg_mask(par, XRX, BITBLT_CNTL, par->bitblt_cntl, COLEXP_MODE);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 814 i740outreg_mask(par, XRX, DISPLAY_CNTL,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 815 par->display_cntl, VGA_WRAP_MODE | GUI_MODE);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 816 i740outreg_mask(par, XRX, PIXPIPE_CONFIG_0, par->pixelpipe_cfg0, 0x9B);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 817 i740outreg_mask(par, XRX, PIXPIPE_CONFIG_2, par->pixelpipe_cfg2, 0x0C);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 818
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 819 i740outreg(par, XRX, PLL_CNTL, par->pll_cntl);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 820
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 821 i740outreg_mask(par, XRX, PIXPIPE_CONFIG_1,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 822 par->pixelpipe_cfg1, DISPLAY_COLOR_MODE);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 823
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 824 itemp = readl(par->regs + FWATER_BLC);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 825 itemp &= ~(LMI_BURST_LENGTH | LMI_FIFO_WATERMARK);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 826 itemp |= par->lmi_fifo_watermark;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 827 writel(itemp, par->regs + FWATER_BLC);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 828
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 829 i740outreg(par, XRX, DRAM_EXT_CNTL, DRAM_REFRESH_60HZ);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 830
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 831 i740outreg_mask(par, MRX, COL_KEY_CNTL_1, 0, BLANK_DISP_OVERLAY);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 832 i740outreg_mask(par, XRX, IO_CTNL,
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 833 par->io_cntl, EXTENDED_ATTR_CNTL | EXTENDED_CRTC_CNTL);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 834
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 835 if (par->pixelpipe_cfg1 != DISPLAY_8BPP_MODE) {
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 836 i740outb(par, VGA_PEL_MSK, 0xFF);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 837 i740outb(par, VGA_PEL_IW, 0x00);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 838 for (i = 0; i < 256; i++) {
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 839 itemp = (par->pixelpipe_cfg0 & DAC_8_BIT) ? i : i >> 2;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 840 i740outb(par, VGA_PEL_D, itemp);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 841 i740outb(par, VGA_PEL_D, itemp);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 842 i740outb(par, VGA_PEL_D, itemp);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 843 }
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 844 }
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 845
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 846 /* Wait for screen to stabilize. */
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 847 mdelay(50);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 848 vga_unprotect(par);
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 849
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 850 info->fix.line_length =
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 851 info->var.xres_virtual * info->var.bits_per_pixel / 8;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 852 if (info->var.bits_per_pixel == 8)
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 853 info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 854 else
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 855 info->fix.visual = FB_VISUAL_TRUECOLOR;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 856
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 857 return 0;
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 858 }
5350c65f4f15bb drivers/video/i740fb.c Ondrej Zary 2012-02-10 859
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 17/17] fortify: Work around Clang inlining bugs
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (15 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 16/17] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
@ 2021-12-13 22:33 ` Kees Cook
2021-12-14 13:22 ` kernel test robot
2021-12-15 0:26 ` [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Jason Gunthorpe
2021-12-17 4:04 ` Martin K. Petersen
18 siblings, 1 reply; 47+ messages in thread
From: Kees Cook @ 2021-12-13 22:33 UTC (permalink / raw)
To: linux-hardening; +Cc: Kees Cook, linux-kernel
To enable FORTIFY_SOURCE support for Clang, the kernel must work around
a pair of bugs, related to Clang's inlining.
Change all the fortified APIs into macros with different inline names to
bypass Clang's broken inline-of-a-builtin detection:
https://bugs.llvm.org/show_bug.cgi?id=50322
Lift all misbehaving __builtin_object_size() calls into the macros to
bypass Clang's broken __builtin_object_size() arguments-of-an-inline
visibility:
https://github.com/ClangBuiltLinux/linux/issues/1401
Thankfully, due to how the inlining already behaves in GCC, this change
has no effect on GCC builds, but allows Clang to finally gain full
FORTIFY coverage.
However, because of a third bug which had no work-arounds, FORTIFY_SOURCE
will only work with Clang version 13 and later. Update the Kconfig to
reflect the new requirements.
Clang 14 introduced compiletime_assert() support, so also adjust the
compile-time warning test to catch Clang's variant of the warning text.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 65 +++++++++++++++++++++-------------
scripts/test_fortify.sh | 8 +++--
security/Kconfig | 2 +-
3 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index c45159dbdaa1..04bcf8307f16 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -50,10 +50,10 @@ extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size)
#define __underlying_strncpy __builtin_strncpy
#endif
-__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
+#define strncpy(p, q, s) __fortify_strncpy(p, q, s, __builtin_object_size(p, 1))
+__FORTIFY_INLINE char *__fortify_strncpy(char *p, const char *q,
+ __kernel_size_t size, size_t p_size)
{
- size_t p_size = __builtin_object_size(p, 1);
-
if (__builtin_constant_p(size) && p_size < size)
__write_overflow();
if (p_size < size)
@@ -73,9 +73,10 @@ __FORTIFY_INLINE char *strcat(char *p, const char *q)
}
extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen);
-__FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
+#define strnlen(p, s) __fortify_strnlen(p, s, __builtin_object_size(p, 1))
+__FORTIFY_INLINE __kernel_size_t __fortify_strnlen(const char *p, size_t maxlen,
+ size_t p_size)
{
- size_t p_size = __builtin_object_size(p, 1);
size_t p_len = __compiletime_strlen(p);
size_t ret;
@@ -94,10 +95,10 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
}
/* defined after fortified strnlen to reuse it. */
-__FORTIFY_INLINE __kernel_size_t strlen(const char *p)
+#define strlen(p) __fortify_strlen(p, __builtin_object_size(p, 1))
+__FORTIFY_INLINE __kernel_size_t __fortify_strlen(const char *p, const size_t p_size)
{
__kernel_size_t ret;
- size_t p_size = __builtin_object_size(p, 1);
/* Give up if we don't know how large p is. */
if (p_size == (size_t)-1)
@@ -110,10 +111,14 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
/* defined after fortified strlen to reuse it */
extern size_t __real_strlcpy(char *, const char *, size_t) __RENAME(strlcpy);
-__FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
+#define strlcpy(p, q, s) __fortify_strlcpy(p, q, s, \
+ __builtin_object_size(p, 1), \
+ __builtin_object_size(q, 1))
+__FORTIFY_INLINE size_t __fortify_strlcpy(char *p, const char *q,
+ size_t size,
+ const size_t p_size,
+ const size_t q_size)
{
- size_t p_size = __builtin_object_size(p, 1);
- size_t q_size = __builtin_object_size(q, 1);
size_t q_len; /* Full count of source string length. */
size_t len; /* Count of characters going into destination. */
@@ -137,12 +142,15 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
/* defined after fortified strnlen to reuse it */
extern ssize_t __real_strscpy(char *, const char *, size_t) __RENAME(strscpy);
-__FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
+#define strscpy(p, q, s) __fortify_strscpy(p, q, s, \
+ __builtin_object_size(p, 1), \
+ __builtin_object_size(q, 1))
+__FORTIFY_INLINE ssize_t __fortify_strscpy(char *p, const char *q,
+ size_t size,
+ const size_t p_size,
+ const size_t q_size)
{
size_t len;
- /* Use string size rather than possible enclosing struct size. */
- size_t p_size = __builtin_object_size(p, 1);
- size_t q_size = __builtin_object_size(q, 1);
/* If we cannot get size of p and q default to call strscpy. */
if (p_size == (size_t) -1 && q_size == (size_t) -1)
@@ -183,11 +191,13 @@ __FORTIFY_INLINE ssize_t strscpy(char *p, const char *q, size_t size)
}
/* defined after fortified strlen and strnlen to reuse them */
-__FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
+#define strncat(p, q, count) __fortify_strncat(p, q, count, \
+ __builtin_object_size(p, 1), \
+ __builtin_object_size(q, 1))
+__FORTIFY_INLINE char *__fortify_strncat(char *p, const char *q, size_t count,
+ size_t p_size, size_t q_size)
{
size_t p_len, copy_len;
- size_t p_size = __builtin_object_size(p, 1);
- size_t q_size = __builtin_object_size(q, 1);
if (p_size == (size_t)-1 && q_size == (size_t)-1)
return __underlying_strncat(p, q, count);
@@ -354,7 +364,8 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
memmove)
extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan);
-__FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
+#define memscan(p, c, s) __fortify_memscan(p, c, s)
+__FORTIFY_INLINE void *__fortify_memscan(void *p, int c, __kernel_size_t size)
{
size_t p_size = __builtin_object_size(p, 0);
@@ -365,7 +376,8 @@ __FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
return __real_memscan(p, c, size);
}
-__FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
+#define memcmp(p, q, s) __fortify_memcmp(p, q, s)
+__FORTIFY_INLINE int __fortify_memcmp(const void *p, const void *q, __kernel_size_t size)
{
size_t p_size = __builtin_object_size(p, 0);
size_t q_size = __builtin_object_size(q, 0);
@@ -381,7 +393,8 @@ __FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
return __underlying_memcmp(p, q, size);
}
-__FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
+#define memchr(p, c, s) __fortify_memchr(p, c, s)
+__FORTIFY_INLINE void *__fortify_memchr(const void *p, int c, __kernel_size_t size)
{
size_t p_size = __builtin_object_size(p, 0);
@@ -393,7 +406,8 @@ __FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
}
void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv);
-__FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
+#define memchr_inv(p, c, s) __fortify_memchr_inv(p, c, s)
+__FORTIFY_INLINE void *__fortify_memchr_inv(const void *p, int c, size_t size)
{
size_t p_size = __builtin_object_size(p, 0);
@@ -417,10 +431,13 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
}
/* Defined after fortified strlen to reuse it. */
-__FORTIFY_INLINE char *strcpy(char *p, const char *q)
+#define strcpy(p, q) __fortify_strcpy(p, q, \
+ __builtin_object_size(p, 1), \
+ __builtin_object_size(q, 1))
+__FORTIFY_INLINE char *__fortify_strcpy(char *p, const char *q,
+ const size_t p_size,
+ const size_t q_size)
{
- size_t p_size = __builtin_object_size(p, 1);
- size_t q_size = __builtin_object_size(q, 1);
size_t size;
/* If neither buffer size is known, immediately give up. */
diff --git a/scripts/test_fortify.sh b/scripts/test_fortify.sh
index a4da365508f0..c2688ab8281d 100644
--- a/scripts/test_fortify.sh
+++ b/scripts/test_fortify.sh
@@ -46,8 +46,12 @@ if "$@" -Werror -c "$IN" -o "$OUT".o 2> "$TMP" ; then
status="warning: unsafe ${FUNC}() usage lacked '$WANT' symbol in $IN"
fi
else
- # If the build failed, check for the warning in the stderr (gcc).
- if ! grep -q -m1 "error: call to .\b${WANT}\b." "$TMP" ; then
+ # If the build failed, check for the warning in the stderr.
+ # GCC:
+ # ./include/linux/fortify-string.h:316:25: error: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror=attribute-warning]
+ # Clang 14:
+ # ./include/linux/fortify-string.h:316:4: error: call to __write_overflow_field declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning]
+ if ! grep -Eq -m1 "error: call to .?\b${WANT}\b.?" "$TMP" ; then
status="warning: unsafe ${FUNC}() usage lacked '$WANT' warning in $IN"
fi
fi
diff --git a/security/Kconfig b/security/Kconfig
index 0b847f435beb..1a25a567965f 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -179,7 +179,7 @@ config FORTIFY_SOURCE
depends on ARCH_HAS_FORTIFY_SOURCE
# https://bugs.llvm.org/show_bug.cgi?id=50322
# https://bugs.llvm.org/show_bug.cgi?id=41459
- depends on !CC_IS_CLANG
+ depends on !CC_IS_CLANG || CLANG_VERSION >= 130000
help
Detect overflows of buffers in common string and memory functions
where the compiler can determine and validate the buffer sizes.
--
2.30.2
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 00/17] Enable strict compile-time memcpy() fortify checks
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (16 preceding siblings ...)
2021-12-13 22:33 ` [PATCH 17/17] fortify: Work around Clang inlining bugs Kees Cook
@ 2021-12-15 0:26 ` Jason Gunthorpe
2021-12-17 4:04 ` Martin K. Petersen
18 siblings, 0 replies; 47+ messages in thread
From: Jason Gunthorpe @ 2021-12-15 0:26 UTC (permalink / raw)
To: Kees Cook; +Cc: linux-hardening, linux-kernel
On Mon, Dec 13, 2021 at 02:33:14PM -0800, Kees Cook wrote:
> Hi,
>
> This is "phase 2" (of several phases) to hardening the kernel against
> memcpy-based buffer overflows. With nearly all compile-time fixes
> landed, the next step is to turn on the warning globally to keep future
> compile-time issues from happening, and let us take the step towards
> run-time checking (and towards a new API for flexible array structures).
>
> This series is based on latest linux-next, and several patches here
> have already been taken by subsystem maintainers but haven't appeared
> in linux-next yet, and are noted below.
I took the RDMA patches to the rdma tree:
> RDMA/mlx5: Use memset_after() to zero struct mlx5_ib_mr
> iw_cxgb4: Use memset_startat() for cpl_t5_pass_accept_rpl
> IB/mthca: Use memset_startat() for clearing mpt_entry
I needed rc5 to come out before I could take the mlx5 patch
Thanks,
Jason
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 00/17] Enable strict compile-time memcpy() fortify checks
2021-12-13 22:33 [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Kees Cook
` (17 preceding siblings ...)
2021-12-15 0:26 ` [PATCH 00/17] Enable strict compile-time memcpy() fortify checks Jason Gunthorpe
@ 2021-12-17 4:04 ` Martin K. Petersen
18 siblings, 0 replies; 47+ messages in thread
From: Martin K. Petersen @ 2021-12-17 4:04 UTC (permalink / raw)
To: Kees Cook, linux-hardening; +Cc: Martin K . Petersen, linux-kernel
On Mon, 13 Dec 2021 14:33:14 -0800, Kees Cook wrote:
> This is "phase 2" (of several phases) to hardening the kernel against
> memcpy-based buffer overflows. With nearly all compile-time fixes
> landed, the next step is to turn on the warning globally to keep future
> compile-time issues from happening, and let us take the step towards
> run-time checking (and towards a new API for flexible array structures).
>
> This series is based on latest linux-next, and several patches here
> have already been taken by subsystem maintainers but haven't appeared
> in linux-next yet, and are noted below.
>
> [...]
Applied to 5.17/scsi-queue, thanks!
[15/17] scsi: lpfc: Use struct_group() to initialize struct lpfc_cgn_info
https://git.kernel.org/mkp/scsi/c/532adda9f405
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 47+ messages in thread