Modify

Opened 7 years ago

Closed 5 years ago

Last modified 4 years ago

#9594 closed defect (wontfix)

Bug in NVRAM manipulation tool (nvram)

Reported by: mikaelbrostrom Owned by: jow
Priority: high Milestone: Chaos Calmer 15.05
Component: packages Version: Trunk
Keywords: nvram bug bricked corrupt staging Cc: mikael_openwrt@…

Description

Hi!

i have found some intresting bugs, that corrupts the staging file /tmp/.nvram, randomly... and if i commit it will write the corrupted file to nvram. and router gets bricked. (serial access and nvram set on those missing values) are needed to unbrick.

if i check inside the corrupted file in /tmp using hexdump it has only 29 of my 306 tuples. the filesize is correct (65536 .nvram)
if i check my nvram mtdblock it has all my 306 tuples.

if i run nvram show | wc -l it says 29.
if i remove the corrupted file and again issue the command.
nvram show | wc -l it says 306.
if i type nvram set testing1234=micke
then a new staging file is created, and everything looks ok again. (show says 307 now)


to replicate the problem, set and unset and commit often. from two places.

tested on following hardware: WRT54GL, WRT54G3G, WRT54GV2, WRT54GV4 same problem.

i think that the problem is due to missing lockfile function.
the old code was in the kernel, the kernel handled the lock and only allowed one nvram cli/lib command to request at once, the other was placed in queue (serialized)..

to fix it:
1: the program should dump nvram directly to the staging file. on startup if executed (with or without arguments). and require a lockfile while it' s doing this.
2: then on all arguments (get,set,show,info) the same, require lock to the staging file.
3: when it commits, lock and write the staging file and NOT unlink() it. and unset the lock.

/Mikael Broström

Attachments (0)

Change History (5)

comment:1 Changed 7 years ago by jow

  • Owner changed from developers to jow
  • Status changed from new to accepted

comment:2 Changed 7 years ago by jow

While I agree that "nvram" could use some locking, I'd say that if you need that kind of frequent access you should probably use libnvram straight away.

comment:3 Changed 7 years ago by mikaelbrostrom

Hi!

well the bug is in libnvram (and nvram since it uses same code)
its the handling of staging file.
if i do the same with a lib, and issue two sets. there will be problems.

i old kernel code 2.4:
https://dev.openwrt.org/browser/trunk/target/linux/brcm-2.4/files/arch/mips/bcm947xx/nvram.c?rev=10137

check for "Should be locked"

same stuff must exist in lib (cli is the same code that uses the lib).

/Mikael Broström

comment:4 Changed 5 years ago by florian

  • Resolution set to wontfix
  • Status changed from accepted to closed

comment:5 Changed 4 years ago by jow

  • Milestone changed from Backfire 10.03.2 to Chaos Calmer (trunk)

Milestone Backfire 10.03.2 deleted

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.