diff mbox series

[1/3] ksmbd: request update to stale share config

Message ID 20220808024341.63913-1-atteh.mailbox@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/3] ksmbd: request update to stale share config | expand

Commit Message

Atte Heikkilä Aug. 8, 2022, 2:43 a.m. UTC
ksmbd_share_config_get() retrieves the cached share config as long
as there is at least one connection to the share. This is an issue when
the user space utilities are used to update share configs. In that case
there is a need to inform ksmbd that it should not use the cached share
config for a new connection to the share. With these changes the tree
connection flag KSMBD_TREE_CONN_FLAG_UPDATE indicates this. When this
flag is set, ksmbd removes the share config from the shares hash table
meaning that ksmbd_share_config_get() ends up requesting a share config
from user space.

Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com>
---
 ksmbd_netlink.h     |  2 ++
 mgmt/share_config.c |  6 +++++-
 mgmt/share_config.h |  1 +
 mgmt/tree_connect.c | 14 ++++++++++++++
 4 files changed, 22 insertions(+), 1 deletion(-)

Comments

Hyunchul Lee Aug. 8, 2022, 4:57 a.m. UTC | #1
Hello atheik,

2022년 8월 8일 (월) 오전 11:47, atheik <atteh.mailbox@gmail.com>님이 작성:
>
> ksmbd_share_config_get() retrieves the cached share config as long
> as there is at least one connection to the share. This is an issue when
> the user space utilities are used to update share configs. In that case
> there is a need to inform ksmbd that it should not use the cached share
> config for a new connection to the share. With these changes the tree
> connection flag KSMBD_TREE_CONN_FLAG_UPDATE indicates this. When this
> flag is set, ksmbd removes the share config from the shares hash table
> meaning that ksmbd_share_config_get() ends up requesting a share config
> from user space.
>
> Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com>
> ---
>  ksmbd_netlink.h     |  2 ++
>  mgmt/share_config.c |  6 +++++-
>  mgmt/share_config.h |  1 +
>  mgmt/tree_connect.c | 14 ++++++++++++++
>  4 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/ksmbd_netlink.h b/ksmbd_netlink.h
> index 192cb13..5d77b72 100644
> --- a/ksmbd_netlink.h
> +++ b/ksmbd_netlink.h
> @@ -349,6 +349,7 @@ enum KSMBD_TREE_CONN_STATUS {
>  #define KSMBD_SHARE_FLAG_STREAMS               BIT(11)
>  #define KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS       BIT(12)
>  #define KSMBD_SHARE_FLAG_ACL_XATTR             BIT(13)
> +#define KSMBD_SHARE_FLAG_UPDATE                BIT(14)
>
>  /*
>   * Tree connect request flags.
> @@ -364,6 +365,7 @@ enum KSMBD_TREE_CONN_STATUS {
>  #define KSMBD_TREE_CONN_FLAG_READ_ONLY         BIT(1)
>  #define KSMBD_TREE_CONN_FLAG_WRITABLE          BIT(2)
>  #define KSMBD_TREE_CONN_FLAG_ADMIN_ACCOUNT     BIT(3)
> +#define KSMBD_TREE_CONN_FLAG_UPDATE            BIT(4)
>
>  /*
>   * RPC over IPC.
> diff --git a/mgmt/share_config.c b/mgmt/share_config.c
> index 70655af..c9bca1c 100644
> --- a/mgmt/share_config.c
> +++ b/mgmt/share_config.c
> @@ -51,12 +51,16 @@ static void kill_share(struct ksmbd_share_config *share)
>         kfree(share);
>  }
>
> -void __ksmbd_share_config_put(struct ksmbd_share_config *share)
> +void ksmbd_share_config_del(struct ksmbd_share_config *share)
>  {
>         down_write(&shares_table_lock);
>         hash_del(&share->hlist);
>         up_write(&shares_table_lock);
> +}
>
> +void __ksmbd_share_config_put(struct ksmbd_share_config *share)
> +{
> +       ksmbd_share_config_del(share);
>         kill_share(share);
>  }
>
> diff --git a/mgmt/share_config.h b/mgmt/share_config.h
> index 28bf351..902f2cb 100644
> --- a/mgmt/share_config.h
> +++ b/mgmt/share_config.h
> @@ -64,6 +64,7 @@ static inline int test_share_config_flag(struct ksmbd_share_config *share,
>         return share->flags & flag;
>  }
>
> +void ksmbd_share_config_del(struct ksmbd_share_config *share);
>  void __ksmbd_share_config_put(struct ksmbd_share_config *share);
>
>  static inline void ksmbd_share_config_put(struct ksmbd_share_config *share)
> diff --git a/mgmt/tree_connect.c b/mgmt/tree_connect.c
> index 7d467e1..0cf6265 100644
> --- a/mgmt/tree_connect.c
> +++ b/mgmt/tree_connect.c
> @@ -65,6 +65,20 @@ ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
>         }
>
>         tree_conn->flags = resp->connection_flags;
> +       if (test_tree_conn_flag(tree_conn, KSMBD_TREE_CONN_FLAG_UPDATE)) {
> +               struct ksmbd_share_config *new_sc;
> +
> +               ksmbd_share_config_del(sc);
> +               new_sc = ksmbd_share_config_get(share_name);
> +               if (!new_sc) {
> +                       pr_err("Failed to update stale share config\n");
> +                       status.ret = -ESTALE;

We need to set proper rsp->hdr.Status for ESTALE in smb2_tree_connect, otherwise
ksmbd will send a response with STATUS_ACCESS_DENIED for this error.

> +                       goto out_error;
> +               }
> +               ksmbd_share_config_put(sc);
> +               sc = new_sc;
> +       }
> +
>         tree_conn->user = sess->user;
>         tree_conn->share_conf = sc;
>         status.tree_conn = tree_conn;
> --
> 2.37.1
>
Namjae Jeon Aug. 8, 2022, 1:06 p.m. UTC | #2
2022-08-08 13:57 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>:
> Hello atheik,
>
> 2022년 8월 8일 (월) 오전 11:47, atheik <atteh.mailbox@gmail.com>님이 작성:
>>
>> ksmbd_share_config_get() retrieves the cached share config as long
>> as there is at least one connection to the share. This is an issue when
>> the user space utilities are used to update share configs. In that case
>> there is a need to inform ksmbd that it should not use the cached share
>> config for a new connection to the share. With these changes the tree
>> connection flag KSMBD_TREE_CONN_FLAG_UPDATE indicates this. When this
>> flag is set, ksmbd removes the share config from the shares hash table
>> meaning that ksmbd_share_config_get() ends up requesting a share config
>> from user space.
>>
>> Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com>
>> ---
>>  ksmbd_netlink.h     |  2 ++
>>  mgmt/share_config.c |  6 +++++-
>>  mgmt/share_config.h |  1 +
>>  mgmt/tree_connect.c | 14 ++++++++++++++
>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/ksmbd_netlink.h b/ksmbd_netlink.h
>> index 192cb13..5d77b72 100644
>> --- a/ksmbd_netlink.h
>> +++ b/ksmbd_netlink.h
>> @@ -349,6 +349,7 @@ enum KSMBD_TREE_CONN_STATUS {
>>  #define KSMBD_SHARE_FLAG_STREAMS               BIT(11)
>>  #define KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS       BIT(12)
>>  #define KSMBD_SHARE_FLAG_ACL_XATTR             BIT(13)
>> +#define KSMBD_SHARE_FLAG_UPDATE                BIT(14)
>>
>>  /*
>>   * Tree connect request flags.
>> @@ -364,6 +365,7 @@ enum KSMBD_TREE_CONN_STATUS {
>>  #define KSMBD_TREE_CONN_FLAG_READ_ONLY         BIT(1)
>>  #define KSMBD_TREE_CONN_FLAG_WRITABLE          BIT(2)
>>  #define KSMBD_TREE_CONN_FLAG_ADMIN_ACCOUNT     BIT(3)
>> +#define KSMBD_TREE_CONN_FLAG_UPDATE            BIT(4)
>>
>>  /*
>>   * RPC over IPC.
>> diff --git a/mgmt/share_config.c b/mgmt/share_config.c
>> index 70655af..c9bca1c 100644
>> --- a/mgmt/share_config.c
>> +++ b/mgmt/share_config.c
>> @@ -51,12 +51,16 @@ static void kill_share(struct ksmbd_share_config
>> *share)
>>         kfree(share);
>>  }
>>
>> -void __ksmbd_share_config_put(struct ksmbd_share_config *share)
>> +void ksmbd_share_config_del(struct ksmbd_share_config *share)
>>  {
>>         down_write(&shares_table_lock);
>>         hash_del(&share->hlist);
>>         up_write(&shares_table_lock);
>> +}
>>
>> +void __ksmbd_share_config_put(struct ksmbd_share_config *share)
>> +{
>> +       ksmbd_share_config_del(share);
>>         kill_share(share);
>>  }
>>
>> diff --git a/mgmt/share_config.h b/mgmt/share_config.h
>> index 28bf351..902f2cb 100644
>> --- a/mgmt/share_config.h
>> +++ b/mgmt/share_config.h
>> @@ -64,6 +64,7 @@ static inline int test_share_config_flag(struct
>> ksmbd_share_config *share,
>>         return share->flags & flag;
>>  }
>>
>> +void ksmbd_share_config_del(struct ksmbd_share_config *share);
>>  void __ksmbd_share_config_put(struct ksmbd_share_config *share);
>>
>>  static inline void ksmbd_share_config_put(struct ksmbd_share_config
>> *share)
>> diff --git a/mgmt/tree_connect.c b/mgmt/tree_connect.c
>> index 7d467e1..0cf6265 100644
>> --- a/mgmt/tree_connect.c
>> +++ b/mgmt/tree_connect.c
>> @@ -65,6 +65,20 @@ ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct
>> ksmbd_session *sess,
>>         }
>>
>>         tree_conn->flags = resp->connection_flags;
>> +       if (test_tree_conn_flag(tree_conn, KSMBD_TREE_CONN_FLAG_UPDATE))
>> {
>> +               struct ksmbd_share_config *new_sc;
>> +
>> +               ksmbd_share_config_del(sc);
>> +               new_sc = ksmbd_share_config_get(share_name);
>> +               if (!new_sc) {
>> +                       pr_err("Failed to update stale share config\n");
>> +                       status.ret = -ESTALE;
>
> We need to set proper rsp->hdr.Status for ESTALE in smb2_tree_connect,
> otherwise
> ksmbd will send a response with STATUS_ACCESS_DENIED for this error.
We can return STATUS_BAD_NETWORK_NAME instead of STATUS_ACCESS_DENIED.

atheik, you can update the below change to your patch. and you need to
create the patch on the latest linux kernel source, not out of tree
ksmbd.

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index d478c3ea4215..233069a37500 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -1944,6 +1944,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
                rsp->hdr.Status = STATUS_SUCCESS;
                rc = 0;
                break;
+       case -ESTALE:
        case -ENOENT:
        case KSMBD_TREE_CONN_STATUS_NO_SHARE:
                rsp->hdr.Status = STATUS_BAD_NETWORK_NAME;

>
>> +                       goto out_error;
>> +               }
>> +               ksmbd_share_config_put(sc);
>> +               sc = new_sc;
>> +       }
>> +
>>         tree_conn->user = sess->user;
>>         tree_conn->share_conf = sc;
>>         status.tree_conn = tree_conn;
>> --
>> 2.37.1
>>
>
>
> --
> Thanks,
> Hyunchul
>
diff mbox series

Patch

diff --git a/ksmbd_netlink.h b/ksmbd_netlink.h
index 192cb13..5d77b72 100644
--- a/ksmbd_netlink.h
+++ b/ksmbd_netlink.h
@@ -349,6 +349,7 @@  enum KSMBD_TREE_CONN_STATUS {
 #define KSMBD_SHARE_FLAG_STREAMS		BIT(11)
 #define KSMBD_SHARE_FLAG_FOLLOW_SYMLINKS	BIT(12)
 #define KSMBD_SHARE_FLAG_ACL_XATTR		BIT(13)
+#define KSMBD_SHARE_FLAG_UPDATE 		BIT(14)
 
 /*
  * Tree connect request flags.
@@ -364,6 +365,7 @@  enum KSMBD_TREE_CONN_STATUS {
 #define KSMBD_TREE_CONN_FLAG_READ_ONLY		BIT(1)
 #define KSMBD_TREE_CONN_FLAG_WRITABLE		BIT(2)
 #define KSMBD_TREE_CONN_FLAG_ADMIN_ACCOUNT	BIT(3)
+#define KSMBD_TREE_CONN_FLAG_UPDATE		BIT(4)
 
 /*
  * RPC over IPC.
diff --git a/mgmt/share_config.c b/mgmt/share_config.c
index 70655af..c9bca1c 100644
--- a/mgmt/share_config.c
+++ b/mgmt/share_config.c
@@ -51,12 +51,16 @@  static void kill_share(struct ksmbd_share_config *share)
 	kfree(share);
 }
 
-void __ksmbd_share_config_put(struct ksmbd_share_config *share)
+void ksmbd_share_config_del(struct ksmbd_share_config *share)
 {
 	down_write(&shares_table_lock);
 	hash_del(&share->hlist);
 	up_write(&shares_table_lock);
+}
 
+void __ksmbd_share_config_put(struct ksmbd_share_config *share)
+{
+	ksmbd_share_config_del(share);
 	kill_share(share);
 }
 
diff --git a/mgmt/share_config.h b/mgmt/share_config.h
index 28bf351..902f2cb 100644
--- a/mgmt/share_config.h
+++ b/mgmt/share_config.h
@@ -64,6 +64,7 @@  static inline int test_share_config_flag(struct ksmbd_share_config *share,
 	return share->flags & flag;
 }
 
+void ksmbd_share_config_del(struct ksmbd_share_config *share);
 void __ksmbd_share_config_put(struct ksmbd_share_config *share);
 
 static inline void ksmbd_share_config_put(struct ksmbd_share_config *share)
diff --git a/mgmt/tree_connect.c b/mgmt/tree_connect.c
index 7d467e1..0cf6265 100644
--- a/mgmt/tree_connect.c
+++ b/mgmt/tree_connect.c
@@ -65,6 +65,20 @@  ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
 	}
 
 	tree_conn->flags = resp->connection_flags;
+	if (test_tree_conn_flag(tree_conn, KSMBD_TREE_CONN_FLAG_UPDATE)) {
+		struct ksmbd_share_config *new_sc;
+
+		ksmbd_share_config_del(sc);
+		new_sc = ksmbd_share_config_get(share_name);
+		if (!new_sc) {
+			pr_err("Failed to update stale share config\n");
+			status.ret = -ESTALE;
+			goto out_error;
+		}
+		ksmbd_share_config_put(sc);
+		sc = new_sc;
+	}
+
 	tree_conn->user = sess->user;
 	tree_conn->share_conf = sc;
 	status.tree_conn = tree_conn;