parisc-isa-eeprom: Fix loff_t usage

loff_t is a signed type. If userspace passes a negative ppos, the "count" range check is weakened. "count"s bigger than HPEE_MAX_LENGTH will pass the check. Also, if ppos is negative, the readb(eisa_eeprom_addr + *ppos) will poke in random memory.

---

Patch is untested due to lack of hardware.

---
 drivers/parisc/eisa_eeprom.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

@@ -48,21 +48,21 @@ static loff_t eisa_eeprom_llseek(struct 
 	return (offset >= 0 && offset < HPEE_MAX_LENGTH) ? (file->f_pos = offset) : -EINVAL;
 }
 
 static ssize_t eisa_eeprom_read(struct file * file,
 			      char __user *buf, size_t count, loff_t *ppos )
 {
 	unsigned char *tmp;
 	ssize_t ret;
 	int i;
 	
-	if (*ppos >= HPEE_MAX_LENGTH)
+	if (*ppos < 0 || *ppos >= HPEE_MAX_LENGTH)
 		return 0;
 	
 	count = *ppos + count < HPEE_MAX_LENGTH ? count : HPEE_MAX_LENGTH - *ppos;
 	tmp = kmalloc(count, GFP_KERNEL);
 	if (tmp) {
 		for (i = 0; i < count; i++)
 			tmp[i] = readb(eisa_eeprom_addr+(*ppos)++);
 
 		if (copy_to_user (buf, tmp, count))
 			ret = -EFAULT;

Greetings, Michael.

Thanks!

Applied and pushed upstream.

Helge

Hang on a minute, this is an untested patch. True, it will likely cause no harm, but it would be more usual to wait for the actual confirmation before declaring the problem fixed.

I'm also very concerned about this:

http://lkml.org/lkml/2009/8/2/107

That's a breach of standard maintainer protocol since you failed to copy the architecture list on the pull request.

Parisc is in a precarious position as a marginal architecture that isn't being produced any more. Having duelling trees and maintainers is definitely very unhelpful because it could cause Linus to lose confidence in our ability as a community.

First things first, you need to agree on a single tree ... although it's perfectly possible to have multiple maintainers commit to it (x86 works this way), can we do this at least before the schizophrenia gets noticed?

Thanks,

James

I'd be shocked if anyone was actually in a position to test this at all... I don't remember where I put my last Mongoose and HP VG ethernet card... It looked fine to me though.

I think Helge was just upset that I wasn't merging things fast enough, and, fair enough, I guess. I promise to rectify that, and if I don't, I plan to step aside.

regards, Kyle

My 725 is sitting idle in the rack ... I think it's got two EISA cards in it. I've also got a Mongoose card for the 715.

There's two problems that would prevent me from testing this.

One is that I don't have any HIL devices, and the current HIL driver goes insane with printks if you have no HIL deviecs plugged in.

The other is that the EISA code refuses to work until the EISA cards have been configured, and we don't have a Linux utility to configure EISA cards.

Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

It just so happens I have a PS2<->HIL thingy doodad around here.

:)

I agree with James. Kyle, your support is crucial, but you don't have to carry the full burden. A single tree that is updated regularly makes it easy for all maintainers to commit to, and for everone to test the changes in the tree that haven't been pushed upstream.

I'm also pushing for stable parisc trees as well if there are enough relevant changes to make this useful.

I know GCC's procedure are different, but it has a single master tree with several hundred maintainers. A maintainer has either global commit, partial commit, or commit after approval priviledge. There is a test requirement for all changes, and all changes have to been sent to the relevant lists. Sometimes things break as a result of a change, but not that often.

I think all the parisc maintainers should be able to push changes upstream, but the list should be notified when this is done.

Dave

J. David Anglin                                  [email protected]
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)

Yes, I sadly missed to copy parisc-mailing-list, but at least I remembered to copy Kyle.

Agreed.

Yep.

Thanks Kyle!
Yes, this was the only reason.

Just a few word on this whole thread, and the unplanned discussion on lkml...

When I sent the pull request, my intention was never to offend Kyle or any other parisc developer. I was even astonished that Kyle offended me suddenly that harsh in public. Neither was my intend to create a duelling tree to the one from Kyle.

I just wanted to make sure, that the latest important patches (e.g. the GOT fix for 64bit kernel modules) just would not miss the 2.6.31 kernel. As we all know, Debian developers lately discussed regularily, if the parisc port should be dropped as a stable/release platform. IMHO, one of the main reasons for the bad state is, that often patches were not merged upstream (or at least not in time), and then backporting them was complicated and often missed.

My pull request included only simple patches. It was planned as a one-time thing just to help Kyle with the maintenance of the outstanding patches. I agree with Kyle, that I should have sent him a notice _before_ sending the pull-request to lkml. Again, I just didn't thought he would be offended by this, esp. since it was just a collection of patches which went to the parisc-list, with a few simple patches by me.

Regarding the push of outstanding patches, I'd really very much prefer a joint tree, to which the maintainers can push. That way everyone can regularily pull a working parisc tree. Furthermore, I'd prefer if we can get the timing right, that most of the outstanding patches goes upstream with a -rc1 release, and then only a minor patchset would be pushed in -rc5 or similar time frame.

Helge

PS: James, welcome as a parisc maintainer! (http://git.kernel.org/?p=linux/kernel/git/kyle/parisc-2.6.git;a=commitdiff;h=4b5273681a72dbf5ece64d0ae1e85d54722012fe)