Page 10 of 43 « First 89 10 111220 Last »
Reply
Thread Tools
iDont is offline iDont
2011-07-09 , 10:27
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#91
Originally Posted by lma View Post
Those problems occur without the patch. Feel free to correct me if I'm missing something, but AFAIK the only problem the patch introduces is not saving history when killed.
This is what I observe after uninstalling busybox-power (and an "apt-get install --reinstall busybox", to be even more sure I'm running Nokia's version):
Shell 1: ~ $ #command 1
Shell 1: ~ $ #command 2

Shell 2: ~ $ #command 3
Shell 2: ~ $ #command 4
Shell 2: ~ $ exit

Shell 1: ~ $ #command 5
Shell 1: ~ $ exit

Shell 3 (root): Nokia-N900:~# cat /home/user/.ash_history
#command 1
#command 2
#command 5
exit
It should be noted that .ash_history contained [#command 3, #command 4, exit] upon exiting shell 2 (as seen by root shell 3). I observed the same behaviour after a recompilation of Nokia's BusyBox, but now with shell-hist.patch left out. Thus, the concurrent shell problem appears to be a problem with the BusyBox 1.10.x series. The next version I've tried (which was the latest when I started busybox-power: 1.18.4) appends each command directly after executing the command to the history file without overwriting another shell's history, causing no problems with concurrent shells.

So you're right about Nokia's patch not introducing the problem. However, it doesn't fix it either.

Regarding your comments on my patch:
Originally Posted by lma View Post
  • In histcopy(), check the result of the input fopen and bail out before clobbering the output file.
Good point, will change that.

Originally Posted by lma View Post
  • I don't agree that if the user doesn't have a ~/.ash_history, or have an empty one, or /tmp is full etc they should be locked out. Return instead of exit?
This was my intention too (see comment: "No history > No shell"), but I seem to have missed changing the exits. histcopy() is based on a regular file copy implementation, which was to be used stand-alone. I'm sure you can figure yourself what went wrong here . Huge thanks for pointing this out to me.

Originally Posted by lma View Post
  • The tmppath directory shouldn't be created with mode 0777 (predictable filenames in world-writable directories are a bit exploitable), 0700 will do.
No excuse for this, will be changed too.

Originally Posted by lma View Post
  • One could trigger a buffer overrun by feeding the shell a long $HOME (though that doesn't give any additional privileges). Using concat_path_file for tmppath would be safer than malloc/strncat here.
Considering it is hard to believe someone has a $HOME > ~235 characters, and the fact that it you can't really gain anything by buffer underrunning, I thought that was an appropriate solution. I will check out concat_path_file for it though, as it can only make the code better.

Thanks for the review. I'll create an v2 when I've got the time for it (probably later today).

Originally Posted by Temporal View Post
May I ask to make it optional? I mean, I edit the .ash_history and put all the things I most use like change frequency or open new windows of filebox & and ping and powertop, and I, for one, prefer that it do not remember what I have done last session unless I hit exit...

BUT I may be happy just with editing my .profile to remove the trap sighup as looks like everybody seems to prefer that it remember and I'm the only one that do not like it much. On the PC I think it's essential, but on my phone, not much.

Thanks!
I think it's better to leave it in, simply because .ash_history is meant to save Ash's history. Retaining incorrect behaviour doesn't seem like the right thing to do.

Just an idea: you could add something like:
Code:
trap 'echo -e "filebox\nping\npowertop\netcetera" > /home/user/.ash_history' 0
to the top of your .profile file (note: the code spans just 1 line). This will restore your .ash_history to whatever you want upon normal shell exit (i.e. without error). You won't have to mind removing the trap line after each busybox-power upgrade then, plus it might even make your life easier
Last edited by iDont; 2011-07-09 at 10:40.
 

The Following 4 Users Say Thank You to iDont For This Useful Post:
lma is offline lma
2011-07-09 , 11:23
Posts: 2,802 | Thanked: 4,491 times | Joined on Nov 2007
#92
Originally Posted by iDont View Post
This is what I observe after uninstalling busybox-power [...]
Aha, you are absolutely correct, I completely missed that all this time :-( I'll look into it.

Considering it is hard to believe someone has a $HOME > ~235 characters, and the fact that it you can't really gain anything by buffer underrunning
Actually I may have been wrong about that, I had forgotten that sudo passes user's $HOME untouched to the root command (bug 5896). I don't know if it's actually exploitable, but better safe than sorry:-)

(Yeah, I know the system is wide open anyway)
 

The Following 3 Users Say Thank You to lma For This Useful Post:
lma is offline lma
2011-07-09 , 12:03
Posts: 2,802 | Thanked: 4,491 times | Joined on Nov 2007
#93
Originally Posted by lma View Post
Aha, you are absolutely correct, I completely missed that all this time :-(
Hm, wrong again. It was pointed out before and fixed for 1.13, but then I completely forgot about it and never back-ported it to 1.6 / 1.10. I need to go have my memory looked at :-(
 

The Following 2 Users Say Thank You to lma For This Useful Post:
iDont is offline iDont
2011-07-09 , 15:22
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#94
Alright, here is V2 of the patch. Changes that went into this new version:
- Fixes for lma's bullets
- Histcopy() is now a static bool. Warning messages are changed too
- Fix in exitshell() - there are cases wherein tmphistory doesn't exist at this point, so check for its existence
- Cleanups in code and CONFIG entries

Again, reviews are highly appreciated. I'll provide a precompiled deb in a minute too. Please let me know if you discover any issues with it.

Update: Attached a new version (V3) of the patch. A more elaborate post will be created later; I'm currently occupied with other stuff. Precompiled deb can be found here.

Edit: this patch is obsolete. A newer version of it has been merged upstream.

---
To clear out possible confusion about the whole issue, and to summarize what this patch is all about:

There are two problems:
Problem 1: BusyBox writes out the shell history after each command, which produces wear on the eMMC.
Problem 2: The default, old BusyBox (1.10.2) had a problem with history contamination

Events:
  • Nokia has made a patch to counter problem 1 by keeping the history in-memory until shell exit.
  • Nokia's patch was based on the old BusyBox of problem 2
  • BusyBox 1.13 fixed problem 2
  • Problem 2 was reintroduced in busybox-power, which uses BusyBox 1.18.5, because we apply (a ported version of) Nokia's patch to fix problem 1.
This new patch effectively does the same as what Nokia's patch did, but targets upstream BusyBox, thus keeping problem 2 fixed, while fixing problem 1. Let's hope everything is clear now, there were some misunderstandings (including by me) in the previous, lengthy posts.
Last edited by iDont; 2011-09-11 at 13:05.
 

The Following 3 Users Say Thank You to iDont For This Useful Post:
qole's Avatar
qole is offline qole
2011-07-09 , 17:17
Moderator | Posts: 7,109 | Thanked: 8,820 times | Joined on Oct 2007 @ Vancouver, BC, Canada
#95
I bet that patch will be useful upstream, too...
__________________
qole.org --- twitter --- Easy Debian wiki page
Please don't send me a private message, post to the appropriate thread.
Thank you all for your donations!
 

The Following 2 Users Say Thank You to qole For This Useful Post:
Estel's Avatar
Estel is offline Estel
2011-07-09 , 23:38
Posts: 5,028 | Thanked: 8,613 times | Joined on Mar 2011
#96
Great work. We can expect this to hit repos ASAP (in fact, when sending anything into them will be possible), right?

Originally Posted by lma
In histcopy(), check the result of the input fopen and bail out before clobbering the output file.
I don't agree that if the user doesn't have a ~/.ash_history, or have an empty one, or /tmp is full etc they should be locked out. Return instead of exit?
The tmppath directory shouldn't be created with mode 0777 (predictable filenames in world-writable directories are a bit exploitable), 0700 will do.
One could trigger a buffer overrun by feeding the shell a long $HOME (though that doesn't give any additional privileges). Using concat_path_file for tmppath would be safer than malloc/strncat here.
I can't keep myself from writing that - reading this post was one of my "epic" moments "What do i know about coding/debian/linux/computers/whatever" Just dropped my jaw into ground.
__________________
N900's aluminum backcover / body replacement
-
N900's HDMI-Out
-
Camera cover MOD
-
Measure battery's real capacity on-device
-
TrueCrypt 7.1 | ereswap | bnf
-
Hardware's mods research is costly. To support my work, please consider donating. Thank You!
 

The Following User Says Thank You to Estel For This Useful Post:
vi_ is offline vi_
2011-07-13 , 12:00
Posts: 1,680 | Thanked: 3,685 times | Joined on Jan 2011
#97
I believe I 'may' have found a bug or something. If I try to cat a file that contains a windblows newline character '^M' I lose everything preceeding that character. I.e. if I try to cat a file containing:

Code:
aaaaaaaaaa^Mbbbbbbbbb^Mccccccccccc^Mdddddddd
I will only see dddddddd as the result. If I remove the newline characters I can see the data.

It makes sed/awk/grep'ing my way to victory really hard!


edit: yay for 'dos2unix'!
__________________
N900: One of God's own prototypes. A high-powered mutant of some kind never even considered for mass production. Too weird to live, and too rare to die.
Last edited by vi_; 2011-07-13 at 12:02.
 

The Following 2 Users Say Thank You to vi_ For This Useful Post:
iDont is offline iDont
2011-07-13 , 14:19
Posts: 268 | Thanked: 1,053 times | Joined on May 2010 @ The Netherlands
#98
Originally Posted by qole View Post
I bet that patch will be useful upstream, too...
I've submitted the patch to BusyBox' mailing list. Discussion can be found here. Please don't mind the text formatting of my emails. I've just switched to Thunderbird, which does everything a little bit different.

At this moment, it seems that there is no upstream interest in the patch. Please refer to the thread over at BusyBox' mailing list for more on this.

Originally Posted by Estel View Post
Great work. We can expect this to hit repos ASAP (in fact, when sending anything into them will be possible), right?
The latest version fo the patch seems to do what it should do, and do just that without any adverse effects. If no issues arise, the patch will pushed to busybox-power's GIT repostory and appear in the next release. I'm currently holding the release off so others can comment on the patch.

Originally Posted by vi_ View Post
I believe I 'may' have found a bug or something. If I try to cat a file that contains a windblows newline character '^M' I lose everything preceeding that character.
Thanks for reporting. However, I can't seem to reproduce this bug over here:
Code:
Nokia-N900:~# echo 'aaaaaaaaaa^Mbbbbbbbbb^Mccccccccccc^Mdddddddd' > testfile && cat testfile 
aaaaaaaaaa^Mbbbbbbbbb^Mccccccccccc^Mdddddddd
Some random text files from a Windows installation posed to be no problem either. Could you attach the specific text file that gave problems on your device? Thanks.

Originally Posted by vi_ View Post
edit: yay for 'dos2unix'!
Guess what provides dos2unix on your device
 

The Following 3 Users Say Thank You to iDont For This Useful Post:
lma is offline lma
2011-07-13 , 14:37
Posts: 2,802 | Thanked: 4,491 times | Joined on Nov 2007
#99
Originally Posted by vi_ View Post
If I try to cat a file that contains a windblows newline character '^M' I lose everything preceeding that character. I.e. if I try to cat a file containing:

Code:
aaaaaaaaaa^Mbbbbbbbbb^Mccccccccccc^Mdddddddd
I will only see dddddddd as the result.
I get ddddddddccc here ;-)

That's normal. ^M is carriage return (aka CR, ASCII 13, '\r'), not newline (LF, ASCII 10, '\n'). Your terminal simply interprets it correctly and moves the output cursor to the beginning of the current line.
 
vi_ is offline vi_
2011-07-13 , 15:08
Posts: 1,680 | Thanked: 3,685 times | Joined on Jan 2011
#100
Code:
curl -A "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3" http://www.metoffice.gov.uk/weather/uk/surface_pressure.html | sed -e 's#<[^>]*>##g' | grep "chilly"
(this may only work for the next 12 hours till the site is updated). The above command should show a line that contains the word chilly (it is a weather report). You can wget the page and look at it in vim to confirm that it does in fact contain the weather report text.

However the above example DOES NOT display the afore mentioned text, it only displays the characters FOLLOWING the last '^M' ion the line. I have checked this by stripping out the yucky '^M's and having it work.

Just so ya know.
__________________
N900: One of God's own prototypes. A high-powered mutant of some kind never even considered for mass production. Too weird to live, and too rare to die.
 
Reply
Page 10 of 43 « First 89 10 111220 Last »

« Previous Thread | Next Thread »
Thread Tools

 
Forum Jump


All times are GMT. The time now is 16:18.

Contact Us - Home - Archive - Top