Messages in this thread | | | Date | Wed, 25 Jan 2012 14:26:29 +0200 | Subject | Re: [GIT] More security subsystem fixes | From | "Kasatkin, Dmitry" <> |
| |
On Thu, Jan 19, 2012 at 4:21 PM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Tetsuo Handa wrote: >> James Morris wrote: >> > MPILIB: Add a missing ENOMEM check >> Maybe one more (shown below) with some random comments. > > Looked a bit more. > > > > In lib/mpi/mpih-div.c: > > mpi_limb_t > mpihelp_divrem(mpi_ptr_t qp, mpi_size_t qextra_limbs, > mpi_ptr_t np, mpi_size_t nsize, mpi_ptr_t dp, mpi_size_t dsize) > { > mpi_limb_t most_significant_q_limb = 0; > > switch (dsize) { > case 0: > /* We are asked to divide by zero, so go ahead and do it! (To make > the compiler not remove this statement, return the value.) */ > return 1 / dsize; > > What's this? Division by 0 in the kernel is no good. > > > > In lib/mpi/mpicoder.c: > > MPI do_encode_md(const void *sha_buffer, unsigned nbits) > { > (...snipped...) > n = 0; > frame[n++] = 0; > frame[n++] = 1; /* block type */ > i = nframe - SHA1_DIGEST_LENGTH - asnlen - 3; > > if (i <= 1) { > pr_info("MPI: message digest encoding failed\n"); > kfree(frame); > return a; > } > > memset(frame + n, 0xff, i); > n += i; > frame[n++] = 0; > memcpy(frame + n, &asn, asnlen); > n += asnlen; > memcpy(frame + n, sha_buffer, SHA1_DIGEST_LENGTH); > n += SHA1_DIGEST_LENGTH; > > i = nframe; > fr_pt = frame; > > if (n != nframe) { > > What's this? i = nframe - SHA1_DIGEST_LENGTH - asnlen - 3; equals > nframe = i + SHA1_DIGEST_LENGTH + asnlen + 3; and this should be always true.
In fact n always equals to nframe, and this check is always false...
> Also, i = nframe; seems to make no sense because i is no longer used. > > printk > ("MPI: message digest encoding failed, frame length is wrong\n"); > kfree(frame); > return a; > } > > a = mpi_alloc((nframe + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB); > > Missing a != NULL check. > > mpi_set_buffer(a, frame, nframe, 0); > kfree(frame); > > return a; > } > > > > In lib/mpi/mpi-pow.c: > > int mpi_powm(MPI res, MPI base, MPI exp, MPI mod) > { > (...snipped...) > if (!msize) > msize = 1 / msize; /* provoke a signal */ > (...snipped...) > } > > Division by 0. > > > > In lib/mpi/mpi-div.c: > > int mpi_tdiv_q_2exp(MPI w, MPI u, unsigned count) > { > (...snipped...) > usize = u->nlimbs; > limb_cnt = count / BITS_PER_MPI_LIMB; > wsize = usize - limb_cnt; > if (limb_cnt >= usize) > w->nlimbs = 0; > else { > mpi_ptr_t wp; > mpi_ptr_t up; > > if (RESIZE_IF_NEEDED(w, wsize) < 0) > return -ENOMEM; > wp = w->d; > up = u->d; > > count %= BITS_PER_MPI_LIMB; > if (count) { > mpihelp_rshift(wp, up + limb_cnt, wsize, count); > wsize -= !wp[wsize - 1]; > (...snipped...) > } > > Is wsize > 0 guaranteed? > Should be, because of the check: if (limb_cnt >= usize) w->nlimbs = 0; else...
> > > Fixes (if any) will be needed to RHEL6's 2.6.32-220.2.1.el6 kernel as well. > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |