diff mbox

Suggested patch: reset errno after isatty()

Message ID AANLkTink+0WvCmQWwd4+Vf8TETjR5-6BsGU=a071Rk0+@mail.gmail.com
State New, archived
Headers show

Commit Message

Ketil Froyn Nov. 17, 2010, 3:50 p.m. UTC 
On Sat, Nov 13, 2010 at 12:55 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Sat, 2010-11-13 at 13:07 +0200, Artem Bityutskiy wrote:
>>
>> however, Mike is right, fixes should be back-ports, or if you fix a
>> problem in the older release, you should fix it in the master branch as
>> well, or show that it does not exist in the master branch. This is the
>> only way it is possible to keep development under control.
>>
>> We really need to be sure that whatever is fixed in older releases is
>> also fixed in the newer. Yes, this is more work for you but hey, I'm
>> also doing this stuff in my spare time.
>
> BTW, with patches from Brian Norris which I just pushed the master
> branch may work better for you.

I've managed to build mtd-utils for my android device, using
codesourcery's toolchain. I built a static nanddump executable, copied
it to my device, and ran it. Here's what happened:

# nanddump -f /sdcard/mtd5.dump /dev/mtd/mtd5
ECC failed: 0
ECC corrected: 0
Number of bad blocks: 0
Number of bbt blocks: 0
Block size 131072, page size 2048, OOB size 0
Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
Segmentation fault

The OOB size shown is 0, which is wrong, while the detected block size
and page size are correct. The old version used to complain:

# nanddump -f /sdcard/mtd5.dump /dev/mtd/mtd5
Unknown flash (not normal NAND)

until I did some debugging and found out that my device has an OOB
size of 56... This patch against v1.4.1 fixed that version:



I've done some digging around in the new code, but I haven't worked
out exactly why the OOB size is set to 0. I haven't figured out
exactly what it does yet... Or could it have anything to do with my
build environment?

Cheers, Ketil

Comments

Ketil Froyn Nov. 18, 2010, 11:13 a.m. UTC | #1 
On Wed, Nov 17, 2010 at 4:50 PM, Ketil Froyn <ketil@froyn.name> wrote:
>
> I've done some digging around in the new code, but I haven't worked
> out exactly why the OOB size is set to 0. I haven't figured out
> exactly what it does yet... Or could it have anything to do with my
> build environment?

I rebuilt nanddump from v1.4.1 with my patches using the CodeSourcery
toolchain (I'd been using the AOSP toolchain before). It works fine,
so I guess that rules out problems in my build environment. Here's
what the run looks like:

/ # nanddump-old -f /sdcard/mtd5.dump /dev/mtd/mtd5
ECC failed: 0
ECC corrected: 0
Number of bad blocks: 0
Number of bbt blocks: 0
Block size 131072, page size 2048, OOB size 56
Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
/ #

This particular system appears to use MTD partitioning, maybe
redboot(?) or something like that. I haven't had the opportunity to
delve into how that works yet - could it be that the partitioning
takes 8 bytes of the OOB?

Cheers, Ketil
Mike Frysinger Nov. 24, 2010, 7:50 a.m. UTC | #2 
On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
> On Wed, Nov 17, 2010 at 4:50 PM, Ketil Froyn wrote:
>> I've done some digging around in the new code, but I haven't worked
>> out exactly why the OOB size is set to 0. I haven't figured out
>> exactly what it does yet... Or could it have anything to do with my
>> build environment?
>
> I rebuilt nanddump from v1.4.1 with my patches using the CodeSourcery
> toolchain (I'd been using the AOSP toolchain before). It works fine,
> so I guess that rules out problems in my build environment. Here's
> what the run looks like:
>
> / # nanddump-old -f /sdcard/mtd5.dump /dev/mtd/mtd5
> ECC failed: 0
> ECC corrected: 0
> Number of bad blocks: 0
> Number of bbt blocks: 0
> Block size 131072, page size 2048, OOB size 56
> Dumping data starting at 0x00000000 and ending at 0x0a5c0000...
> / #

so you'll want to dive into the ioctls to see what isnt working right.
 strace is your friend.

> This particular system appears to use MTD partitioning, maybe
> redboot(?) or something like that. I haven't had the opportunity to
> delve into how that works yet - could it be that the partitioning
> takes 8 bytes of the OOB?

no
-mike
Ketil Froyn Nov. 24, 2010, 9:59 a.m. UTC | #3 
On Wed, Nov 24, 2010 at 8:50 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
>
> so you'll want to dive into the ioctls to see what isnt working right.
>  strace is your friend.

Thanks for the kick. I had tried that, but for some reason strace was
dying with the error "syscall: unknown syscall trap 0xe8bd8008". A
little more searching now led me to this patch, which fixed it for me:

http://www.fluff.org/ben/patches/strace/strace-fix-arm-bad-syscall.patch

Anyway, here's the full output:

execve("/system/nanddump", ["/system/nanddump", "-f", "/sdcard/dump",
"/dev/mtd/mtd5"], [/* 6 vars */]) = 0
uname({sys="Linux", node="localhost", ...}) = 0
brk(0)                                  = 0x94000
brk(0x94d00)                            = 0x94d00
syscall_983045(0x944c0, 0x917d0, 0xffffffe0, 0x10, 0x92048, 0x8, 0x10,
0xf0005, 0x9185c, 0x4, 0x28, 0x944c0, 0, 0xbeccad00, 0xe474, 0xe484,
0x40000010, 0x944c0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
brk(0xb5d00)                            = 0xb5d00
brk(0xb6000)                            = 0xb6000
open("/sys/class/mtd", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY|0x80000) = 3
fcntl64(3, F_GETFD)                     = 0x1 (flags FD_CLOEXEC)
pivot_root(0x3, "")                     = 384
close(3)                                = 0
open("/sys/class/mtd/mtd0/name", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No
such file or directory)
open("/dev/mtd/mtd5", O_RDONLY|O_LARGEFILE) = 3
stat64("/dev/mtd/mtd5", {st_mode=S_IFCHR|0600, st_rdev=makedev(90,
10), ...}) = 0
open("/dev/mtd/mtd5", O_RDWR|O_LARGEFILE) = 4
ioctl(4, 0x80204d01, 0xbecca978)        = 0
ioctl(4, 0x40084d0b, 0xbecca998)        = 0
close(4)                                = 0
open("/proc/mtd", O_RDONLY|O_LARGEFILE) = 4
read(4, "dev:    size   erasesize  name\nmtd0: 00040000 00020000
\"misc\"\nmtd1: 00500000 00020000 \"recovery\"\nmtd2: 00280000
00020000 \"boot\"\nmtd3: 0aa00000 00020000 \"system\"\nmtd4: 08200000
00020000 \"cache\"\nmtd5: 0a5c0000 00020000 \"userdata\"\n", 4096) =
228
close(4)                                = 0
ioctl(3, 0x80104d12, 0xbeccacfc)        = 0
write(2, "ECC failed: 0\n", 14)         = 14
write(2, "ECC corrected: 0\n", 17)      = 17
write(2, "Number of bad blocks: 0\n", 24) = 24
write(2, "Number of bbt blocks: 0\n", 24) = 24
open("/sdcard/dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbecca9ec) = -1 ENOTTY (Not a
typewriter)
write(2, "Block size 131072, page size 2048, OOB size 0\n", 46) = 46
write(2, "Dumping data starting at 0x00000000 and ending at
0x0a5c0000...\n", 64) = 64
ioctl(3, 0x40084d0b, 0xbeccaa48)        = 0
_llseek(3, 0, [0], SEEK_SET)            = 0
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeccacec)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc0184d16, 0xbecca9e8)        = -1 ENOTTY (Not a typewriter)
ioctl(3, 0xc00c4d04 <unfinished ...>
+++ killed by SIGSEGV +++

The old version of nanddump (with my patches) works fine on the same
system. It looks like this:

execve("/system/nanddump-old", ["/system/nanddump-old", "-f",
"/sdcard/old-dump", "/dev/mtd/mtd5"], [/* 6 vars */]) = 0
uname({sys="Linux", node="localhost", ...}) = 0
brk(0)                                  = 0x96000
brk(0x96d00)                            = 0x96d00
syscall_983045(0x964c0, 0x92010, 0xffffffe0, 0x10, 0x939a0, 0x8, 0x10,
0xf0005, 0x920c8, 0x4, 0x28, 0x964c0, 0, 0xbeeb6cf0, 0xb0b4, 0xb0c4,
0x40000010, 0x964c0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) = 0
brk(0xb7d00)                            = 0xb7d00
brk(0xb8000)                            = 0xb8000
open("/dev/mtd/mtd5", O_RDONLY|O_LARGEFILE) = 3
ioctl(3, 0x80204d01, 0xbeeb6ca4)        = 0
ioctl(3, 0x80104d12, 0xbeeb6cd4)        = 0
write(2, "ECC failed: 0\n", 14)         = 14
write(2, "ECC corrected: 0\n", 17)      = 17
write(2, "Number of bad blocks: 0\n", 24) = 24
write(2, "Number of bbt blocks: 0\n", 24) = 24
open("/sdcard/old-dump", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0644) = 4
ioctl(4, TCGETS or SNDCTL_TMR_TIMEBASE, 0xbeeb6aec) = -1 ENOTTY (Not a
typewriter)
write(2, "Block size 131072, page size 2048, OOB size 56\n", 47) = 47
write(2, "Dumping data starting at 0x00000000 and ending at
0x0a5c0000...\n", 64) = 64
ioctl(3, 0x40084d0b, 0xbeeb6cf0)        = 0
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeeb6cc4)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc00c4d04, 0xbeeb6ce4)        = 0
write(4, "[snip]", 56) = 56
read(3, "[snip]"..., 2048) = 2048
ioctl(3, 0x80104d12, 0xbeeb6cc4)        = 0
write(4, "[snip]"..., 2048) = 2048
ioctl(3, 0xc00c4d04, 0xbeeb6ce4)        = 0
write(4, "[snip]", 56) = 56
etc...

>> This particular system appears to use MTD partitioning, maybe
>> redboot(?) or something like that. I haven't had the opportunity to
>> delve into how that works yet - could it be that the partitioning
>> takes 8 bytes of the OOB?
>
> no

Thanks for the clarification. I guess those are the actual physical
parameters, then.

Cheers, Ketil
Artem Bityutskiy Nov. 24, 2010, 2:12 p.m. UTC | #4 
On Wed, 2010-11-24 at 10:59 +0100, Ketil Froyn wrote:
> On Wed, Nov 24, 2010 at 8:50 AM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> > On Thu, Nov 18, 2010 at 06:13, Ketil Froyn wrote:
> >
> > so you'll want to dive into the ioctls to see what isnt working right.
> >  strace is your friend.
> 
> Thanks for the kick. I had tried that, but for some reason strace was
> dying with the error "syscall: unknown syscall trap 0xe8bd8008". A
> little more searching now led me to this patch, which fixed it for me:
> 
> http://www.fluff.org/ben/patches/strace/strace-fix-arm-bad-syscall.patch
> 
> Anyway, here's the full output:

So it dies very soon. You should easily find the line of code where this
happens with gdb. Just compile nanddump with -g -O0

But I guess we are calling some new ioctl on the system which does not
support it, and do not fall-back to the new ioctl. We tried to be
careful about this, though.
diff mbox

Patch

--- a/nanddump.c
+++ b/nanddump.c
@@ -308,6 +308,7 @@  int main(int argc, char * const argv[])
                        !(meminfo.oobsize == 128 && meminfo.writesize == 4096) &
                        !(meminfo.oobsize == 64 && meminfo.writesize == 4096) &&
                        !(meminfo.oobsize == 64 && meminfo.writesize == 2048) &&
+                       !(meminfo.oobsize == 56 && meminfo.writesize == 2048) &&
                        !(meminfo.oobsize == 32 && meminfo.writesize == 1024) &&
                        !(meminfo.oobsize == 16 && meminfo.writesize == 512) &&
                        !(meminfo.oobsize == 8 && meminfo.writesize == 256)) {