On Sun, 2021-08-22 at 09:00 +0000, THOBY Simon wrote:
> On 8/22/21 10:55 AM, THOBY Simon wrote:
> > The new function validate_hash_algo() assumed that ima_get_hash_algo()
> > always return a valid 'enum hash_algo', but it returned the
> > user-supplied value present in the digital signature without
> > any bounds checks.
> >
> > Update ima_get_hash_algo() to always return a valid hash algorithm,
> > defaulting on 'ima_hash_algo' when the user-supplied value inside
> > the xattr is invalid.
> >
> > Signed-off-by: THOBY Simon <Simon.THOBY@xxxxxxxxxx>
> > Reported-by: syzbot+e8bafe7b82c739eaf153@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Fixes: 50f742dd9147 ("IMA: block writes of the security.ima xattr with
> > unsupported algorithms")
> > Reviewed-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
> > ---
> > security/integrity/ima/ima_appraise.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> > index 8f1eb7ef041e..dbba51583e7c 100644
> > --- a/security/integrity/ima/ima_appraise.c
> > +++ b/security/integrity/ima/ima_appraise.c
> > @@ -185,7 +185,8 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
> > switch (xattr_value->type) {
> > case EVM_IMA_XATTR_DIGSIG:
> > sig = (typeof(sig))xattr_value;
> > - if (sig->version != 2 || xattr_len <= sizeof(*sig))
> > + if (sig->version != 2 || xattr_len <= sizeof(*sig)
> > + || sig->hash_algo >= HASH_ALGO__LAST)
> > return ima_hash_algo;
> > return sig->hash_algo;
> > break;
> >
>
> Oops, I forgot to update the patch version, and to add a changelog.
> Here it is:
> - Updating the commit message
> - Adding the 'Fixes:' and 'Reviewed-by:' annotations
>
> As the patch content didn't change, the comment on
> syszbot ("This patch was successfully tested by syszbot, see
> https://syzkaller.appspot.com/bug?extid=e8bafe7b82c739eaf153.";)
> is still true.
>
> Sorry about that,
This looks fine. Comments that you want to convey to
reviewers/maintainers, like the testing info, go after the patch
description after three-dash line. Only the information that should
be retained should be in the patch description.
thanks,
Mimi
