| Message ID | 20240123002814.1396804-80-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | None | expand |
On 23.01.2024 01:27, Kees Cook wrote: > --- a/drivers/net/xen-netback/hash.c > +++ b/drivers/net/xen-netback/hash.c > @@ -345,7 +345,7 @@ u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len, > .flags = GNTCOPY_source_gref > }}; > > - if ((off + len < off) || (off + len > vif->hash.size) || > + if ((add_would_overflow(off, len)) || (off + len > vif->hash.size) || I'm not maintainer of this code, but if I was I would ask that the excess parentheses be removed, to improve readability. Jan
On Tue, Jan 23, 2024 at 08:55:44AM +0100, Jan Beulich wrote: > On 23.01.2024 01:27, Kees Cook wrote: > > --- a/drivers/net/xen-netback/hash.c > > +++ b/drivers/net/xen-netback/hash.c > > @@ -345,7 +345,7 @@ u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len, > > .flags = GNTCOPY_source_gref > > }}; > > > > - if ((off + len < off) || (off + len > vif->hash.size) || > > + if ((add_would_overflow(off, len)) || (off + len > vif->hash.size) || > > I'm not maintainer of this code, but if I was I would ask that the > excess parentheses be removed, to improve readability. Good call. I will adjust that. Thanks! -Kees
diff --git a/drivers/net/xen-netback/hash.c b/drivers/net/xen-netback/hash.c index ff96f22648ef..69b03b4feba9 100644 --- a/drivers/net/xen-netback/hash.c +++ b/drivers/net/xen-netback/hash.c @@ -345,7 +345,7 @@ u32 xenvif_set_hash_mapping(struct xenvif *vif, u32 gref, u32 len, .flags = GNTCOPY_source_gref }}; - if ((off + len < off) || (off + len > vif->hash.size) || + if ((add_would_overflow(off, len)) || (off + len > vif->hash.size) || len > XEN_PAGE_SIZE / sizeof(*mapping)) return XEN_NETIF_CTRL_STATUS_INVALID_PARAMETER;
In an effort to separate intentional arithmetic wrap-around from unexpected wrap-around, we need to refactor places that depend on this kind of math. One of the most common code patterns of this is: VAR + value < VAR Notably, this is considered "undefined behavior" for signed and pointer types, which the kernel works around by using the -fno-strict-overflow option in the build[1] (which used to just be -fwrapv). Regardless, we want to get the kernel source to the position where we can meaningfully instrument arithmetic wrap-around conditions and catch them when they are unexpected, regardless of whether they are signed[2], unsigned[3], or pointer[4] types. Refactor open-coded wrap-around addition test to use add_would_overflow(). This paves the way to enabling the wrap-around sanitizers in the future. Link: https://git.kernel.org/linus/68df3755e383e6fecf2354a67b08f92f18536594 [1] Link: https://github.com/KSPP/linux/issues/26 [2] Link: https://github.com/KSPP/linux/issues/27 [3] Link: https://github.com/KSPP/linux/issues/344 [4] Cc: Wei Liu <wei.liu@kernel.org> Cc: Paul Durrant <paul@xen.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: xen-devel@lists.xenproject.org Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/net/xen-netback/hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)