There is bug which lead to an infinite loop. In the next

Fixed in <a class="issue-link js-issue-link" data-error-text="Failed to load title" da

Infinite loop on NtfsAttributes custom iterator next about ntfs HOT 3 CLOSED

colinfinck commented on July 28, 2024

Infinite loop on NtfsAttributes custom iterator next

from ntfs.

Comments (3)

ColinFinck commented on July 28, 2024 1

Hey @Arthur-PI! Thanks for opening this bug report.
As you already saw, I fixed it a bit differently in #23, but an explanation is still missing. So here it comes: Zero-byte seeks are totally legit and also happen very often when parsing healthy filesystems. In fact, .seek(SeekFrom::Current(0)) has been the official way to determine the current seek position in Rust for a long time until they introduced .stream_position() in 1.51.0.
As a result, we can't fix this problem by treating zero-byte seeks as an error.

Instead, I've added code to reset stream_data_run and data_position when seeking beyond the data covered by Data Runs (for non-resident attributes). Previously, attributes could lie about their total length, and your test48 file did exactly that (u64::MAX was chosen for length). This also means that the "infinite loop" was finite after all, just taking VERY long until all Data Runs were seeked (and working with a stale data_position in the process -- definitely a serious bug!). The new code bails out early now as soon as we seeked through all available Data Runs and none is left.

Your test48 file also has a Data Run with zero cluster_count but non-zero vcn, which is also invalid. I've added a validation step to also return a specific error in this case.

from ntfs.

Pastequee commented on July 28, 2024

News! After some digging, turns out the problem is in the seek implementation of NtfsNonResidentAttributeValue here :

ntfs/src/attribute_value/non_resident.rs

Lines 149 to 193 in 9348d72

 fn seek<T>(&mut self, fs: &mut T, pos: SeekFrom) -> Result<u64> 

 where 

 T: Read + Seek, 

 { 

 let pos = self.stream_state.optimize_seek(pos, self.len())?; 

 let mut bytes_left_to_seek = match pos { 

 SeekFrom::Start(n) => { 

 self.rewind()?; 

 n 

 } 

 SeekFrom::Current(n) if n >= 0 => n as u64, 

 _ => unreachable!(), 

 }; 

 while bytes_left_to_seek > 0 { 

 // Seek inside the current Data Run if there is one. 

 if self 

 .stream_state 

 .seek_data_run(fs, pos, &mut bytes_left_to_seek)? 

 { 

 // We have reached our final seek position. 

 break; 

 } 

 // Move to the next Data Run. 

 if self.next_data_run()? { 

 // We got another Data Run, so seek some more. 

 continue; 

 } else { 

 // We seeked as far as we could. 

 break; 

 } 

 } 

 match pos { 

 SeekFrom::Start(n) => self.stream_state.set_stream_position(n), 

 SeekFrom::Current(n) => self 

 .stream_state 

 .set_stream_position(self.stream_position() + n as u64), 

 _ => unreachable!(), 

 } 

 Ok(self.stream_position()) 

 }

And the problem is that bytes_left_to_seek is equal it 0 and so it does not seek a thing, so the state of NonResidentAttributeValue never change and so on...
So I have implemented a simple fix once again but I doubt that this is is the preferred solution for you, you tell me.
So my solution is to just add before the while bytes_left_to_seek > 0

if bytes_left_to_seek == 0 {
    return Err(NtfsError::NoBytesToSeekNonResidentValue {});
}

With a new custom error that need to be decide

from ntfs.

ColinFinck commented on July 28, 2024

Fixed in #23

from ntfs.

Infinite loop on NtfsAttributes custom iterator next about ntfs HOT 3 CLOSED

Comments (3)

Related Issues (19)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	fn seek<T>(&mut self, fs: &mut T, pos: SeekFrom) -> Result<u64>
	where
	T: Read + Seek,
	{
	let pos = self.stream_state.optimize_seek(pos, self.len())?;

	let mut bytes_left_to_seek = match pos {
	SeekFrom::Start(n) => {
	self.rewind()?;
	n
	}
	SeekFrom::Current(n) if n >= 0 => n as u64,
	_ => unreachable!(),
	};

	while bytes_left_to_seek > 0 {
	// Seek inside the current Data Run if there is one.
	if self
	.stream_state
	.seek_data_run(fs, pos, &mut bytes_left_to_seek)?
	{
	// We have reached our final seek position.
	break;
	}

	// Move to the next Data Run.
	if self.next_data_run()? {
	// We got another Data Run, so seek some more.
	continue;
	} else {
	// We seeked as far as we could.
	break;
	}
	}

	match pos {
	SeekFrom::Start(n) => self.stream_state.set_stream_position(n),
	SeekFrom::Current(n) => self
	.stream_state
	.set_stream_position(self.stream_position() + n as u64),
	_ => unreachable!(),
	}

	Ok(self.stream_position())
	}