linux-kernel - Re: [PATCH v3 2/2] kexec: Use min

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: <20130227143133.7208d381.akpm@linux-foundation.org>
Date:	Wed, 27 Feb 2013 14:31:33 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Zhang Yanfei <zhangyanfei@...fujitsu.com>
Cc:	Joe Perches <joe@...ches.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Simon Horman <horms@...ge.net.au>,
	"kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] kexec: Use min_t to simplify logic

On Tue, 26 Feb 2013 16:55:57 +0800
Zhang Yanfei <zhangyanfei@...fujitsu.com> wrote:

> ___ 2013___02___26___ 16:53, Andrew Morton ______:
> > On Tue, 26 Feb 2013 16:49:02 +0800 Zhang Yanfei <zhangyanfei@...fujitsu.com> wrote:
> > 
> >>>> diff --git a/kernel/kexec.c b/kernel/kexec.c
> >>> []
> >>>> @@ -822,13 +822,9 @@ static int kimage_load_normal_segment(struct kimage *image,
> >>> []
> >>>> +		mchunk = min_t(size_t, mbytes,
> >>>> +			       (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
> >>>
> >>> #define min_t(type, x, y) ({			\
> >>> 	type __min1 = (x);			\
> >>> 	type __min2 = (y);			\
> >>> 	__min1 < __min2 ? __min1: __min2; })
> >>>
> >>>
> >>>
> >>
> >> Hmm, from the definition, the cast is redundant.
> >>
> >> Maybe I misunderstood what Andrew meant in the mail related to v2:
> >>
> >> "The types of PAGE_SIZE and PAGE_MASK are vague - iirc they once had
> >> different types on different architectures, so some form of casting is
> >> unavoidable here."
> >>
> >> Andrew, could you please explain the casting you meant above?
> > 
> > I mean that a cast (or min_t, which is a cast) will be needed. 
> > The code you have here casts the same thing two times, which isn't
> > necessary.
> > 
> > 
> 
> Thanks.
> 
> Should I resend the patch again? After removing this redundant cast,
> it is the same with v2.

That patch was pretty busted.   This should fix everything up:

--- a/kernel/kexec.c~kexec-use-min_t-to-simplify-logic-fix
+++ a/kernel/kexec.c
@@ -820,8 +820,8 @@ static int kimage_load_normal_segment(st
 		clear_page(ptr);
 		ptr += maddr & ~PAGE_MASK;
 		mchunk = min_t(size_t, mbytes,
-			       (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
-		uchunk = min_t(size_t, ubytes, mchunk);
+				PAGE_SIZE - (maddr & ~PAGE_MASK));
+		uchunk = min(ubytes, mchunk);
 
 		result = copy_from_user(ptr, buf, uchunk);
 		kunmap(page);
@@ -868,8 +868,8 @@ static int kimage_load_crash_segment(str
 		ptr = kmap(page);
 		ptr += maddr & ~PAGE_MASK;
 		mchunk = min_t(size_t, mbytes,
-			       (size_t)(PAGE_SIZE - (maddr & ~PAGE_MASK)));
-		uchunk = min_t(size_t, ubytes, mchunk);
+				PAGE_SIZE - (maddr & ~PAGE_MASK));
+		uchunk = min(ubytes, mchunk);
 		if (mchunk > uchunk) {
 			/* Zero the trailing part of the page */
 			memset(ptr + uchunk, 0, mchunk - uchunk);
@@ -1451,7 +1451,7 @@ void vmcoreinfo_append_str(const char *f
 	r = vsnprintf(buf, sizeof(buf), fmt, args);
 	va_end(args);
 
-	r = min_t(size_t, r, vmcoreinfo_max_size - vmcoreinfo_size);
+	r = min(r, vmcoreinfo_max_size - vmcoreinfo_size);
 
 	memcpy(&vmcoreinfo_data[vmcoreinfo_size], buf, r);
 
_

--
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/