linux-kernel - Re: [GIT] More security subsystem fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201201190620.q0J6KMvA047997@www262.sakura.ne.jp>
Date:	Thu, 19 Jan 2012 15:20:22 +0900
From:	from-linux-security-module@...ove.sakura.ne.jp
To:	jmorris@...ei.org
Cc:	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org
Subject: Re: [GIT] More security subsystem fixes 

James Morris wrote:
>       MPILIB: Add a missing ENOMEM check
Maybe one more (shown below) with some random comments.



        for (i = a->nlimbs - 1; i >= 0; i--) {
                alimb = a->d[i];
#if BYTES_PER_MPI_LIMB == 4
                *p++ = alimb >> 24;
                *p++ = alimb >> 16;
                *p++ = alimb >> 8;
                *p++ = alimb;
#elif BYTES_PER_MPI_LIMB == 8
                *p++ = alimb >> 56;
                *p++ = alimb >> 48;
                *p++ = alimb >> 40;
                *p++ = alimb >> 32;
                *p++ = alimb >> 24;
                *p++ = alimb >> 16;
                *p++ = alimb >> 8;
                *p++ = alimb;
#else
#error please implement for this limb size.
#endif

Are such large bitshift guaranteed to work?



MPI do_encode_md(const void *sha_buffer, unsigned nbits)
{
(...snipped...)
        a = mpi_alloc((nframe + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB);
	// Missing a != NULL check here.
        mpi_set_buffer(a, frame, nframe, 0);
        kfree(frame);
(...snipped...)
}

Is (nframe + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB * sizeof(mpi_limb_t)
guaranteed not to overflow? Is nlimbs guaranteed to be not 0?

MPI mpi_alloc(unsigned nlimbs)
{
	mpi_alloc_limb_space(nlimbs)
	{
		mpi_ptr_t mpi_alloc_limb_space(unsigned nlimbs)
		{
		        size_t len = nlimbs * sizeof(mpi_limb_t);
		
		        return kmalloc(len, GFP_KERNEL);
		}
	}
}

There are several kmalloc(nlimbs * sizeof(mpi_limb_t)) calls.
Are they guaranteed not to overflow?

int mpi_resize(MPI a, unsigned nlimbs)
{
        void *p;

        if (nlimbs <= a->alloced)
                return 0;       /* no need to do it */

        if (a->d) {
		// Can use krealloc()?
                p = kmalloc(nlimbs * sizeof(mpi_limb_t), GFP_KERNEL);
                if (!p)
                        return -ENOMEM;
                memcpy(p, a->d, a->alloced * sizeof(mpi_limb_t));
                kfree(a->d);
                a->d = p;
        } else {
                a->d = kzalloc(nlimbs * sizeof(mpi_limb_t), GFP_KERNEL);
                if (!a->d)
                        return -ENOMEM;
        }
        a->alloced = nlimbs;
        return 0;
}

Roughly browsing, it seems to me that this module assumes that kmalloc()
returns NULL if requested size was 0. It is OK for userspace, but I think we
should introduce and use a version of kmalloc() that returns NULL if requested
size was 0 (so that we can catch 0 byte allocation unless overflow).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/