From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: Re: [linux-next:master 6552/7920] net/sunrpc/auth_gss/gss_krb5_mech.c:254:38: warning: Invalid test for overflow 'p+20<p'. Condition is always false unless there is overflow, and overflow is undefined behaviour.
Date: Wed, 16 Sep 2020 16:19:36 +0300 [thread overview]
Message-ID: <20200916131936.GI18329@kadam> (raw)
In-Reply-To: <202009161341.mJoDy6IE%lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 8694 bytes --]
On Wed, Sep 16, 2020 at 01:57:47PM +0800, kernel test robot wrote:
> CC: kbuild-all(a)lists.01.org
> TO: Ard Biesheuvel <ardb@kernel.org>
> CC: Herbert Xu <herbert@gondor.apana.org.au>
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: 6b02addb1d1748d21dd1261e46029b264be4e5a0
> commit: e33d2a7b3041d7f8cd1f0a2a4ca42a5bc112b14e [6552/7920] SUNRPC: remove RC4-HMAC-MD5 support from KerberosV
> :::::: branch date: 22 hours ago
> :::::: commit date: 5 days ago
> compiler: s390-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
>
> cppcheck warnings: (new ones prefixed by >>)
>
> >> net/sunrpc/auth_gss/gss_krb5_keys.c:219:6: warning: Variable 'ret' is reassigned a value before the old one has been used. [redundantAssignment]
> ret = (*(gk5e->mk_key))(gk5e, &inblock, outkey);
> ^
> net/sunrpc/auth_gss/gss_krb5_keys.c:168:6: note: Variable 'ret' is reassigned a value before the old one has been used.
> ret = ENOMEM;
> ^
> net/sunrpc/auth_gss/gss_krb5_keys.c:219:6: note: Variable 'ret' is reassigned a value before the old one has been used.
> ret = (*(gk5e->mk_key))(gk5e, &inblock, outkey);
> ^
> >> net/sunrpc/auth_gss/gss_krb5_mech.c:254:38: warning: Invalid test for overflow 'p+20<p'. Condition is always false unless there is overflow, and overflow is undefined behaviour. [invalidTestForOverflow]
> if (unlikely(p + 20 > end || p + 20 < p)) {
In the kernel we use a GCC flag to make these overflows defined
behavior. Lots of places rely on it to work.
regards,
dan carpenter
> ^
>
> # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=e33d2a7b3041d7f8cd1f0a2a4ca42a5bc112b14e
> git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> git fetch --no-tags linux-next master
> git checkout e33d2a7b3041d7f8cd1f0a2a4ca42a5bc112b14e
> vim +254 net/sunrpc/auth_gss/gss_krb5_mech.c
>
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 229
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 230 static int
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 231 gss_import_v1_context(const void *p, const void *end, struct krb5_ctx *ctx)
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 232 {
> c3be6577d82a9f Paul Burton 2018-11-01 233 u32 seq_send;
> e678e06bf8fa25 J. Bruce Fields 2006-12-04 234 int tmp;
> 294ec5b87a8aae Arnd Bergmann 2018-06-07 235 u32 time32;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 236
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 237 p = simple_get_bytes(p, end, &ctx->initiate, sizeof(ctx->initiate));
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 238 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 239 goto out_err;
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 240
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 241 /* Old format supports only DES! Any other enctype uses new format */
> 1ac3719a2214c5 Kevin Coffman 2010-03-17 242 ctx->enctype = ENCTYPE_DES_CBC_RAW;
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 243
> 81d4a4333a1dfd Kevin Coffman 2010-03-17 244 ctx->gk5e = get_gss_krb5_enctype(ctx->enctype);
> ce8477e1176389 Bian Naimeng 2010-09-12 245 if (ctx->gk5e == NULL) {
> ce8477e1176389 Bian Naimeng 2010-09-12 246 p = ERR_PTR(-EINVAL);
> 81d4a4333a1dfd Kevin Coffman 2010-03-17 247 goto out_err;
> ce8477e1176389 Bian Naimeng 2010-09-12 248 }
> 81d4a4333a1dfd Kevin Coffman 2010-03-17 249
> 717757ad1038ab J. Bruce Fields 2006-12-04 250 /* The downcall format was designed before we completely understood
> 717757ad1038ab J. Bruce Fields 2006-12-04 251 * the uses of the context fields; so it includes some stuff we
> 717757ad1038ab J. Bruce Fields 2006-12-04 252 * just give some minimal sanity-checking, and some we ignore
> 717757ad1038ab J. Bruce Fields 2006-12-04 253 * completely (like the next twenty bytes): */
> ce8477e1176389 Bian Naimeng 2010-09-12 @254 if (unlikely(p + 20 > end || p + 20 < p)) {
> ce8477e1176389 Bian Naimeng 2010-09-12 255 p = ERR_PTR(-EFAULT);
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 256 goto out_err;
> ce8477e1176389 Bian Naimeng 2010-09-12 257 }
> 717757ad1038ab J. Bruce Fields 2006-12-04 258 p += 20;
> e678e06bf8fa25 J. Bruce Fields 2006-12-04 259 p = simple_get_bytes(p, end, &tmp, sizeof(tmp));
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 260 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 261 goto out_err;
> ef338bee3f4f50 Kevin Coffman 2007-11-09 262 if (tmp != SGN_ALG_DES_MAC_MD5) {
> ef338bee3f4f50 Kevin Coffman 2007-11-09 263 p = ERR_PTR(-ENOSYS);
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 264 goto out_err;
> ef338bee3f4f50 Kevin Coffman 2007-11-09 265 }
> d922a84a8bf1d6 J. Bruce Fields 2006-12-04 266 p = simple_get_bytes(p, end, &tmp, sizeof(tmp));
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 267 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 268 goto out_err;
> ef338bee3f4f50 Kevin Coffman 2007-11-09 269 if (tmp != SEAL_ALG_DES) {
> ef338bee3f4f50 Kevin Coffman 2007-11-09 270 p = ERR_PTR(-ENOSYS);
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 271 goto out_err;
> ef338bee3f4f50 Kevin Coffman 2007-11-09 272 }
> 294ec5b87a8aae Arnd Bergmann 2018-06-07 273 p = simple_get_bytes(p, end, &time32, sizeof(time32));
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 274 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 275 goto out_err;
> 294ec5b87a8aae Arnd Bergmann 2018-06-07 276 /* unsigned 32-bit time overflows in year 2106 */
> 294ec5b87a8aae Arnd Bergmann 2018-06-07 277 ctx->endtime = (time64_t)time32;
> c3be6577d82a9f Paul Burton 2018-11-01 278 p = simple_get_bytes(p, end, &seq_send, sizeof(seq_send));
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 279 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 280 goto out_err;
> c3be6577d82a9f Paul Burton 2018-11-01 281 atomic_set(&ctx->seq_send, seq_send);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 282 p = simple_get_netobj(p, end, &ctx->mech_used);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 283 if (IS_ERR(p))
> a8cc1cb7d7a12b Kevin Coffman 2010-03-17 284 goto out_err;
> 81d4a4333a1dfd Kevin Coffman 2010-03-17 285 p = get_key(p, end, ctx, &ctx->enc);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 286 if (IS_ERR(p))
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 287 goto out_err_free_mech;
> 81d4a4333a1dfd Kevin Coffman 2010-03-17 288 p = get_key(p, end, ctx, &ctx->seq);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 289 if (IS_ERR(p))
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 290 goto out_err_free_key1;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 291 if (p != end) {
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 292 p = ERR_PTR(-EFAULT);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 293 goto out_err_free_key2;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 294 }
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 295
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 296 return 0;
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 297
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 298 out_err_free_key2:
> e9e575b8f29445 Kees Cook 2018-09-18 299 crypto_free_sync_skcipher(ctx->seq);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 300 out_err_free_key1:
> e9e575b8f29445 Kees Cook 2018-09-18 301 crypto_free_sync_skcipher(ctx->enc);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 302 out_err_free_mech:
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 303 kfree(ctx->mech_used.data);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 304 out_err:
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 305 return PTR_ERR(p);
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 306 }
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 307
>
> :::::: The code at line 254 was first introduced by commit
> :::::: ce8477e1176389ed920550f4c925ad4a815b22d5 gss:krb5 miss returning error to caller when import security context
>
> :::::: TO: Bian Naimeng <biannm@cn.fujitsu.com>
> :::::: CC: Trond Myklebust <Trond.Myklebust@netapp.com>
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
> _______________________________________________
> kbuild mailing list -- kbuild(a)lists.01.org
> To unsubscribe send an email to kbuild-leave(a)lists.01.org
prev parent reply other threads:[~2020-09-16 13:19 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 5:57 [linux-next:master 6552/7920] net/sunrpc/auth_gss/gss_krb5_mech.c:254:38: warning: Invalid test for overflow 'p+20<p'. Condition is always false unless there is overflow, and overflow is undefined behaviour kernel test robot
2020-09-16 13:19 ` Dan Carpenter [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200916131936.GI18329@kadam \
--to=dan.carpenter@oracle.com \
--cc=kbuild@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.