Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] - App crashes when creating clients from multiple threads #17

Open
qarmin opened this issue Apr 11, 2024 · 3 comments
Open

[BUG] - App crashes when creating clients from multiple threads #17

qarmin opened this issue Apr 11, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@qarmin
Copy link
Contributor

qarmin commented Apr 11, 2024

Description

When creating multiple clients from multiple threads, app crashes with info like realloc(): invalid next size
code

use std::fs;
use log::warn;
use pavao::{SmbClient, SmbCredentials, SmbError, SmbMode, SmbOpenOptions, SmbOptions};
use std::io::Write;
use std::thread::sleep;
use std::time::Duration;
use rayon::prelude::*;

fn main() {
    let file_content = fs::read("/home/rafal/Obrazy/Screenshots/Screenshot from 2024-04-05 12-42-02.png").unwrap();
    let localhost = "smb://localhost/sambashare2";

    (0..100).into_par_iter().for_each(|_| {
        let client = create_smblient(localhost, "user", "user").unwrap();
    });
}

pub(crate) fn create_smblient(address: &str, username: &str, password: &str) -> Result<SmbClient, SmbError> {
    let username = if username.trim().is_empty() {
        "username".to_string()
    } else {
        username.trim().to_string()
    };

    let password = if password.trim().is_empty() {
        "password".to_string()
    } else {
        password.trim().to_string()
    };

    SmbClient::new(
        SmbCredentials::default()
            .server(&address)
            .username(&username)
            .password(&password),
        SmbOptions::default(),
    )
}

Project - testt.zip

valgrind shows such errors

Freed frame ../../source3/param/loadparm.c:3961, expected ../../source3/libsmb/libsmb_context.c:50.
Freed frame ../../source3/libsmb/libsmb_context.c:142, expected ../../source3/libsmb/libsmb_context.c:50.
Freed frame ../../source3/libsmb/libsmb_context.c:50, expected ../../source3/libsmb/libsmb_context.c:50.
==335011== Thread 4:
==335011== Invalid write of size 1
==335011==    at 0x49133DE: lp_do_section (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4C93679: tini_parse (in /usr/lib/x86_64-linux-gnu/libsamba-util.so.0.0.1)
==335011==    by 0x4C94FFD: pm_process (in /usr/lib/x86_64-linux-gnu/libsamba-util.so.0.0.1)
==335011==    by 0x4916000: ??? (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4916823: lp_load_global (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4869EC9: ??? (in /usr/lib/x86_64-linux-gnu/libsmbclient.so.0.7.0)
==335011==    by 0x486E436: smbc_new_context (in /usr/lib/x86_64-linux-gnu/libsmbclient.so.0.7.0)
==335011==    by 0x15291E: pavao::smb::client::SmbClient::new (client.rs:71)
==335011==    by 0x11D864: testt::create_smblient (main.rs:33)
==335011==    by 0x11DCA5: testt::main::{{closure}} (main.rs:14)
==335011==    by 0x11DC68: core::ops::function::impls::<impl core::ops::function::FnMut<A> for &F>::call_mut (function.rs:272)
==335011==    by 0x11D959: core::iter::traits::iterator::Iterator::for_each::call::{{closure}} (iterator.rs:855)
==335011==  Address 0x90da848 is 152 bytes inside a block of size 192 free'd
==335011==    at 0x4E08337: free (vg_replace_malloc.c:985)
==335011==    by 0x4DA12A3: ??? (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4DA4A0B: talloc_free_children (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4C92C27: ??? (in /usr/lib/x86_64-linux-gnu/libsamba-util.so.0.0.1)
==335011==    by 0x4DA14B7: ??? (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4DA4A0B: talloc_free_children (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4C92C27: ??? (in /usr/lib/x86_64-linux-gnu/libsamba-util.so.0.0.1)
==335011==    by 0x4DA14B7: ??? (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4DA4A0B: talloc_free_children (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4C92C27: ??? (in /usr/lib/x86_64-linux-gnu/libsamba-util.so.0.0.1)
==335011==    by 0x4DA14B7: ??? (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4DA4A0B: talloc_free_children (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==  Block was alloc'd at
==335011==    at 0x4E050C5: malloc (vg_replace_malloc.c:442)
==335011==    by 0x4DA1D7F: ??? (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4DA209A: _talloc_zero (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.3.3)
==335011==    by 0x4D01418: loadparm_init_s3 (in /usr/lib/x86_64-linux-gnu/libsamba-hostconfig.so.0.0.1)
==335011==    by 0x49069FD: ??? (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4915DCA: ??? (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4916823: lp_load_global (in /usr/lib/x86_64-linux-gnu/libsmbconf.so.0.0.1)
==335011==    by 0x4869EC9: ??? (in /usr/lib/x86_64-linux-gnu/libsmbclient.so.0.7.0)
==335011==    by 0x486E436: smbc_new_context (in /usr/lib/x86_64-linux-gnu/libsmbclient.so.0.7.0)
==335011==    by 0x15291E: pavao::smb::client::SmbClient::new (client.rs:71)
==335011==    by 0x11D864: testt::create_smblient (main.rs:33)
==335011==    by 0x11DCA5: testt::main::{{closure}} (main.rs:14)

I want to connect to different servers, basing on user input from different threads - should I use some cache/lock mechanism or this code is fine, but there is bug in this repo?

Environment

  • OS: Ubuntu 22.04
  • Architecture x86_64
  • Rust version: 1.77.2
  • libsmbclient - 4.15, but also using 4.10 on arm device
@qarmin qarmin added the bug Something isn't working label Apr 11, 2024
@qarmin
Copy link
Contributor Author

qarmin commented Apr 15, 2024

Changing

pavao/src/smb/client.rs

Lines 59 to 90 in e538105

impl SmbClient {
/// Initialize a new `SmbClient` with the provided credentials to connect to the remote smb server
pub fn new(credentials: SmbCredentials, options: SmbOptions) -> SmbResult<Self> {
let uri = Self::build_uri(credentials.server.as_str(), credentials.share.as_str());
let smbc = SmbClient { uri };
// insert credentials
trace!("creating context...");
// get current context
let is_ctx_null = SMBCTX.lock().map_err(|_| SmbError::Mutex)?.is_null();
// if context is null, create a new one
if is_ctx_null {
unsafe {
let ctx = utils::result_from_ptr_mut(smbc_new_context())?;
// set options
trace!("configuring client options");
smbc_setFunctionAuthDataWithContext(ctx, Some(Self::auth_wrapper));
Self::setup_options(ctx, options);
// set ctx
let smb_ctx = utils::result_from_ptr_mut(smbc_init_context(ctx))?;
trace!("context initialized");
AUTH_SERVICE
.lock()
.unwrap()
.insert(Self::auth_service_uuid(smb_ctx), credentials);
// set context
SMBCTX.lock().map_err(|_| SmbError::Mutex)?.set(smb_ctx);
}
}
Ok(smbc)
}

into

impl SmbClient {
    /// Initialize a new `SmbClient` with the provided credentials to connect to the remote smb server
    pub fn new(credentials: SmbCredentials, options: SmbOptions) -> SmbResult<Self> {
        let uri = Self::build_uri(credentials.server.as_str(), credentials.share.as_str());
        let smbc = SmbClient { uri };
        // insert credentials
        trace!("creating context...");
        // get current context
        let lock = SMBCTX.lock().map_err(|_| SmbError::Mutex)?;
        // if context is null, create a new one
        if lockis_null() {
            unsafe {
                let ctx = utils::result_from_ptr_mut(smbc_new_context())?;
                // set options
                trace!("configuring client options");
                smbc_setFunctionAuthDataWithContext(ctx, Some(Self::auth_wrapper));
                Self::setup_options(ctx, options);

                // set ctx
                let smb_ctx = utils::result_from_ptr_mut(smbc_init_context(ctx))?;
                trace!("context initialized");
                AUTH_SERVICE
                    .lock()
                    .unwrap()
                    .insert(Self::auth_service_uuid(smb_ctx), credentials);

                // set context
                lock.set(smb_ctx);
            }
        }
        Ok(smbc)
    }

helps a little(removes crashes from Client::new functions), but later threaded(and also possibly non threaded) operations like mkdir still fails.

More and more I think it might be limitation of libsmbclient

@evilsocket
Copy link

@qarmin
Copy link
Contributor Author

qarmin commented Jul 19, 2024

This code crashes app, so this is problem of libsmbclient.

use rayon::prelude::*;
#[repr(C)]
struct SMBCCTX {
    _private: [u8; 0],
}

extern "C" {
    fn smbc_new_context() -> *mut SMBCCTX;
}

fn crash() {
    (0..100).into_par_iter().for_each(|_| {
        unsafe {
            smbc_new_context();
        }
    });
}

fn main() {
    crash();
}

in build.rs

fn main() {
    println!("cargo:rustc-link-lib=smbclient");
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants