On Wed, Dec 20, 2023 at 5:31 PM Aishwarya TCV <aishwarya.tcv@xxxxxxx> wrote:
> On 24/10/2023 22:35, Paul Moore wrote:
> > While we have a lsm_fill_user_ctx() helper function designed to make
> > life easier for LSMs which return lsm_ctx structs to userspace, we
> > didn't include all of the buffer length safety checks and buffer
> > padding adjustments in the helper. This led to code duplication
> > across the different LSMs and the possibility for mistakes across the
> > different LSM subsystems. In order to reduce code duplication and
> > decrease the chances of silly mistakes, we're consolidating all of
> > this code into the lsm_fill_user_ctx() helper.
> >
> > The buffer padding is also modified from a fixed 8-byte alignment to
> > an alignment that matches the word length of the machine
> > (BITS_PER_LONG / 8).
> >
> > Signed-off-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> > ---
> > include/linux/security.h | 9 ++++---
> > security/apparmor/lsm.c | 15 +++--------
> > security/security.c | 55 +++++++++++++++++++++-----------------
> > security/selinux/hooks.c | 42 +++++++++++++++--------------
> > security/smack/smack_lsm.c | 23 +++++-----------
> > 5 files changed, 67 insertions(+), 77 deletions(-)
>
> Hi Paul,
>
> While building the kernel against next-master for arch arm64
> > security/security.c:810:2: warning: ‘memcpy’ offset 32 is out of the bounds [0, 0] [-Warray-bounds]
> warning is observed. On some other architectures like i386 and x86_64,
> an error is observed. > arch/x86/include/asm/string_32.h:150:25: error:
> ‘__builtin_memcpy’ offset 32 is out of the bounds [0, 0]
> [-Werror=array-bounds]
I believe the code is correct, I'm guessing this is simply a question
of the compiler not seeing whatever syntactic magic is required for
your compilation flags. While I'm not entirely sure of the "[0, 0]"
"bounds" in the warning/error message, if that were a
offset/limit/length a double zero value would also seem to indicate
this is more of a compiler annotation issue than a code issue.
Looking at the lsm_ctx definition in include/uapi/linux/lsm.h I see
the following:
struct lsm_ctx {
__u64 id; /* offset: 0 */
__u64 flags; /* offset: 8 */
__u64 len; /* offset: 16 */
__u64 ctx_len; /* offset: 24 */
__u8 ctx[]; /* offset: 32 */
};
and given that the offending line of code is trying to do a memcpy
into the ctx field, an offset of 32 looks correct to me.
Suggestions on how to annotate the struct, or the code doing the
memcpy() are welcome.
--
paul-moore.com
