Re: [GIT] More security subsystem fixes

[Tetsuo Handa]

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.
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?



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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/