lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 25 Oct 2022 11:45:19 -0700 From: Kees Cook <keescook@...omium.org> To: Dave Chinner <david@...morbit.com> Cc: "Darrick J. Wong" <djwong@...nel.org>, xfs <linux-xfs@...r.kernel.org>, Zorro Lang <zlang@...hat.com>, linux-hardening@...r.kernel.org Subject: Re: [RFC PATCH] xfs: fix FORTIFY_SOURCE complaints about log item memcpy On Tue, Oct 25, 2022 at 09:32:35AM +1100, Dave Chinner wrote: > On Mon, Oct 24, 2022 at 09:59:08AM -0700, Kees Cook wrote: > > On Wed, Oct 19, 2022 at 05:04:11PM -0700, Darrick J. Wong wrote: > > > [...] > > > -/* > > > - * Copy an BUI format buffer from the given buf, and into the destination > > > - * BUI format structure. The BUI/BUD items were designed not to need any > > > - * special alignment handling. > > > - */ > > > -static int > > > -xfs_bui_copy_format( > > > - struct xfs_log_iovec *buf, > > > - struct xfs_bui_log_format *dst_bui_fmt) > > > -{ > > > - struct xfs_bui_log_format *src_bui_fmt; > > > - uint len; > > > - > > > - src_bui_fmt = buf->i_addr; > > > - len = xfs_bui_log_format_sizeof(src_bui_fmt->bui_nextents); > > > - > > > - if (buf->i_len == len) { > > > - memcpy(dst_bui_fmt, src_bui_fmt, len); > > > - return 0; > > > - } > > > - XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL); > > > - return -EFSCORRUPTED; > > > -} > > > > This is the place where flex_cpy() could be used: > > > > flex_cpy(dst_bui_fmt, src_bui_fmt); > > How does flex_cpy() know how much memory was allocated for > dst_bui_fmt? Doesn't knowing this imply that we have to set the > count field in dst_bui_fmt appropriately before flex_cpy() is > called? Right -- this is why I had originally sent my API proposal with the *_dup helpers included as well. The much more common case is allocate/copy and allocate/deserialize. The case of doing flex-to-flex is odd, because it implies there was an external allocation step, etc. But, that said, allocation and bounds recording are usually pretty well tied together. > Hence I don't see that this flex array copying stuff will make it > harder to make mistakes, but ISTM that it'll make them harder to spot > during review and audit... I think the transition to a fallible routine is an improvement, but yes, I expect flex_cpy() not to be used much compared to flex_dup(), or mem_to_flex_dup(), both of which collapse a great many steps and sanity checks into a single common, internally-consistent, and fallible operation. -- Kees Cook
Powered by blists - more mailing lists