* [Cluster-devel] [bug report] fs: dlm: trace user space callbacks
@ 2022-08-29 7:41 Dan Carpenter
2022-08-29 13:06 ` Alexander Aring
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-08-29 7:41 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hello Alexander Aring,
The patch 7a3de7324c2b: "fs: dlm: trace user space callbacks" from
Aug 15, 2022, leads to the following Smatch static checker warning:
fs/dlm/lock.c:5900 dlm_user_request()
warn: 'lkb' was already freed.
fs/dlm/lock.c
5832 int dlm_user_request(struct dlm_ls *ls, struct dlm_user_args *ua,
5833 int mode, uint32_t flags, void *name, unsigned int namelen)
5834 #endif
5835 {
5836 struct dlm_lkb *lkb;
5837 struct dlm_args args;
5838 int error;
5839
5840 dlm_lock_recovery(ls);
5841
5842 error = create_lkb(ls, &lkb);
5843 if (error) {
5844 kfree(ua);
5845 goto out;
5846 }
5847
5848 trace_dlm_lock_start(ls, lkb, name, namelen, mode, flags);
5849
5850 if (flags & DLM_LKF_VALBLK) {
5851 ua->lksb.sb_lvbptr = kzalloc(DLM_USER_LVB_LEN, GFP_NOFS);
5852 if (!ua->lksb.sb_lvbptr) {
5853 kfree(ua);
5854 __put_lkb(ls, lkb);
5855 error = -ENOMEM;
5856 goto out_trace_end;
5857 }
5858 }
5859 #ifdef CONFIG_DLM_DEPRECATED_API
5860 error = set_lock_args(mode, &ua->lksb, flags, namelen, timeout_cs,
5861 fake_astfn, ua, fake_bastfn, &args);
5862 #else
5863 error = set_lock_args(mode, &ua->lksb, flags, namelen, fake_astfn, ua,
5864 fake_bastfn, &args);
5865 #endif
5866 if (error) {
5867 kfree(ua->lksb.sb_lvbptr);
5868 ua->lksb.sb_lvbptr = NULL;
5869 kfree(ua);
5870 __put_lkb(ls, lkb);
5871 goto out_trace_end;
5872 }
5873
5874 /* After ua is attached to lkb it will be freed by dlm_free_lkb().
5875 When DLM_IFL_USER is set, the dlm knows that this is a userspace
5876 lock and that lkb_astparam is the dlm_user_args structure. */
5877 lkb->lkb_flags |= DLM_IFL_USER;
5878 error = request_lock(ls, lkb, name, namelen, &args);
5879
5880 switch (error) {
5881 case 0:
5882 break;
5883 case -EINPROGRESS:
5884 error = 0;
5885 break;
5886 case -EAGAIN:
5887 error = 0;
5888 fallthrough;
5889 default:
5890 __put_lkb(ls, lkb);
5891 goto out_trace_end;
5892 }
5893
5894 /* add this new lkb to the per-process list of locks */
5895 spin_lock(&ua->proc->locks_spin);
5896 hold_lkb(lkb);
5897 list_add_tail(&lkb->lkb_ownqueue, &ua->proc->locks);
5898 spin_unlock(&ua->proc->locks_spin);
5899 out_trace_end:
--> 5900 trace_dlm_lock_end(ls, lkb, name, namelen, mode, flags, error, false);
^^^
This is freed, but probably the trace code doesn't care? I'm not sure.
5901 out:
5902 dlm_unlock_recovery(ls);
5903 return error;
5904 }
5905
5906 #ifdef CONFIG_DLM_DEPRECATED_API
5907 int dlm_user_convert(struct dlm_ls *ls, struct dlm_user_args *ua_tmp,
5908 int mode, uint32_t flags, uint32_t lkid, char *lvb_in,
5909 unsigned long timeout_cs)
5910 #else
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* [Cluster-devel] [bug report] fs: dlm: trace user space callbacks
2022-08-29 7:41 [Cluster-devel] [bug report] fs: dlm: trace user space callbacks Dan Carpenter
@ 2022-08-29 13:06 ` Alexander Aring
0 siblings, 0 replies; 2+ messages in thread
From: Alexander Aring @ 2022-08-29 13:06 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On Mon, Aug 29, 2022 at 3:41 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Alexander Aring,
>
> The patch 7a3de7324c2b: "fs: dlm: trace user space callbacks" from
> Aug 15, 2022, leads to the following Smatch static checker warning:
>
> fs/dlm/lock.c:5900 dlm_user_request()
> warn: 'lkb' was already freed.
>
> fs/dlm/lock.c
> 5832 int dlm_user_request(struct dlm_ls *ls, struct dlm_user_args *ua,
> 5833 int mode, uint32_t flags, void *name, unsigned int namelen)
> 5834 #endif
> 5835 {
> 5836 struct dlm_lkb *lkb;
> 5837 struct dlm_args args;
> 5838 int error;
> 5839
> 5840 dlm_lock_recovery(ls);
> 5841
> 5842 error = create_lkb(ls, &lkb);
> 5843 if (error) {
> 5844 kfree(ua);
> 5845 goto out;
> 5846 }
> 5847
> 5848 trace_dlm_lock_start(ls, lkb, name, namelen, mode, flags);
> 5849
> 5850 if (flags & DLM_LKF_VALBLK) {
> 5851 ua->lksb.sb_lvbptr = kzalloc(DLM_USER_LVB_LEN, GFP_NOFS);
> 5852 if (!ua->lksb.sb_lvbptr) {
> 5853 kfree(ua);
> 5854 __put_lkb(ls, lkb);
> 5855 error = -ENOMEM;
> 5856 goto out_trace_end;
> 5857 }
> 5858 }
> 5859 #ifdef CONFIG_DLM_DEPRECATED_API
> 5860 error = set_lock_args(mode, &ua->lksb, flags, namelen, timeout_cs,
> 5861 fake_astfn, ua, fake_bastfn, &args);
> 5862 #else
> 5863 error = set_lock_args(mode, &ua->lksb, flags, namelen, fake_astfn, ua,
> 5864 fake_bastfn, &args);
> 5865 #endif
> 5866 if (error) {
> 5867 kfree(ua->lksb.sb_lvbptr);
> 5868 ua->lksb.sb_lvbptr = NULL;
> 5869 kfree(ua);
> 5870 __put_lkb(ls, lkb);
> 5871 goto out_trace_end;
> 5872 }
> 5873
> 5874 /* After ua is attached to lkb it will be freed by dlm_free_lkb().
> 5875 When DLM_IFL_USER is set, the dlm knows that this is a userspace
> 5876 lock and that lkb_astparam is the dlm_user_args structure. */
> 5877 lkb->lkb_flags |= DLM_IFL_USER;
> 5878 error = request_lock(ls, lkb, name, namelen, &args);
> 5879
> 5880 switch (error) {
> 5881 case 0:
> 5882 break;
> 5883 case -EINPROGRESS:
> 5884 error = 0;
> 5885 break;
> 5886 case -EAGAIN:
> 5887 error = 0;
> 5888 fallthrough;
> 5889 default:
> 5890 __put_lkb(ls, lkb);
> 5891 goto out_trace_end;
> 5892 }
> 5893
> 5894 /* add this new lkb to the per-process list of locks */
> 5895 spin_lock(&ua->proc->locks_spin);
> 5896 hold_lkb(lkb);
> 5897 list_add_tail(&lkb->lkb_ownqueue, &ua->proc->locks);
> 5898 spin_unlock(&ua->proc->locks_spin);
> 5899 out_trace_end:
> --> 5900 trace_dlm_lock_end(ls, lkb, name, namelen, mode, flags, error, false);
> ^^^
> This is freed, but probably the trace code doesn't care? I'm not sure.
>
Thanks. It cares, there is currently an issue. We need to rearrange
the code there so we don't have any use after free here.
However the user has to opt-in.
- Alex
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-08-29 13:06 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-29 7:41 [Cluster-devel] [bug report] fs: dlm: trace user space callbacks Dan Carpenter
2022-08-29 13:06 ` Alexander Aring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.