Skip to content

Commit

Permalink
afs: Fix tracepoint string placement with built-in AFS
Browse files Browse the repository at this point in the history 
To quote Alexey[1]:

    I was adding custom tracepoint to the kernel, grabbed full F34 kernel
    .config, disabled modules and booted whole shebang as VM kernel.

    Then did

	perf record -a -e ...

    It crashed:

	general protection fault, probably for non-canonical address 0x435f5346592e4243: 0000 [#1] SMP PTI
	CPU: 1 PID: 842 Comm: cat Not tainted 5.12.6+ #26
	Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-1.fc33 04/01/2014
	RIP: 0010:t_show+0x22/0xd0

    Then reproducer was narrowed to

	# cat /sys/kernel/tracing/printk_formats

    Original F34 kernel with modules didn't crash.

    So I started to disable options and after disabling AFS everything
    started working again.

    The root cause is that AFS was placing char arrays content into a
    section full of _pointers_ to strings with predictable consequences.

    Non canonical address 435f5346592e4243 is "CB.YFS_" which came from
    CM_NAME macro.

    Steps to reproduce:

	CONFIG_AFS=y
	CONFIG_TRACING=y

	# cat /sys/kernel/tracing/printk_formats

Fix this by the following means:

 (1) Add enum->string translation tables in the event header with the AFS
     and YFS cache/callback manager operations listed by RPC operation ID.

 (2) Modify the afs_cb_call tracepoint to print the string from the
     translation table rather than using the string at the afs_call name
     pointer.

 (3) Switch translation table depending on the service we're being accessed
     as (AFS or YFS) in the tracepoint print clause.  Will this cause
     problems to userspace utilities?

     Note that the symbolic representation of the YFS service ID isn't
     available to this header, so I've put it in as a number.  I'm not sure
     if this is the best way to do this.

 (4) Remove the name wrangling (CM_NAME) macro and put the names directly
     into the afs_call_type structs in cmservice.c.

Fixes: 8e8d7f1 ("afs: Add some tracepoints")
Reported-by: Alexey Dobriyan (SK hynix) <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Steven Rostedt (VMware) <[email protected]>
Reviewed-by: Marc Dionne <[email protected]>
cc: Andrew Morton <[email protected]>
cc: [email protected]
Link: https://lore.kernel.org/r/YLAXfvZ+rObEOdc%[email protected]/ [1]
Link: https://lore.kernel.org/r/[email protected]/
Link: https://lore.kernel.org/r/162430903582.2896199.6098150063997983353.stgit@warthog.procyon.org.uk/ # v1
Link: https://lore.kernel.org/r/162609463957.3133237.15916579353149746363.stgit@warthog.procyon.org.uk/ # v1 (repost)
Link: https://lore.kernel.org/r/162610726860.3408253.445207609466288531.stgit@warthog.procyon.org.uk/ # v2
  • Loading branch information
@dhowells
dhowells committed Jul 21, 2021
1 parent e73f0f0 commit 6c881ca
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 23 deletions.
25 changes: 7 additions & 18 deletions fs/afs/cmservice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,11 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *);

static int afs_deliver_yfs_cb_callback(struct afs_call *);

#define CM_NAME(name) \
char afs_SRXCB##name##_name[] __tracepoint_string = \
"CB." #name

/*
* CB.CallBack operation type
*/
static CM_NAME(CallBack);
static const struct afs_call_type afs_SRXCBCallBack = {
.name = afs_SRXCBCallBack_name,
.name = "CB.CallBack",
.deliver = afs_deliver_cb_callback,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_CallBack,
Expand All @@ -47,9 +42,8 @@ static const struct afs_call_type afs_SRXCBCallBack = {
/*
* CB.InitCallBackState operation type
*/
static CM_NAME(InitCallBackState);
static const struct afs_call_type afs_SRXCBInitCallBackState = {
.name = afs_SRXCBInitCallBackState_name,
.name = "CB.InitCallBackState",
.deliver = afs_deliver_cb_init_call_back_state,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_InitCallBackState,
Expand All @@ -58,9 +52,8 @@ static const struct afs_call_type afs_SRXCBInitCallBackState = {
/*
* CB.InitCallBackState3 operation type
*/
static CM_NAME(InitCallBackState3);
static const struct afs_call_type afs_SRXCBInitCallBackState3 = {
.name = afs_SRXCBInitCallBackState3_name,
.name = "CB.InitCallBackState3",
.deliver = afs_deliver_cb_init_call_back_state3,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_InitCallBackState,
Expand All @@ -69,9 +62,8 @@ static const struct afs_call_type afs_SRXCBInitCallBackState3 = {
/*
* CB.Probe operation type
*/
static CM_NAME(Probe);
static const struct afs_call_type afs_SRXCBProbe = {
.name = afs_SRXCBProbe_name,
.name = "CB.Probe",
.deliver = afs_deliver_cb_probe,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_Probe,
Expand All @@ -80,9 +72,8 @@ static const struct afs_call_type afs_SRXCBProbe = {
/*
* CB.ProbeUuid operation type
*/
static CM_NAME(ProbeUuid);
static const struct afs_call_type afs_SRXCBProbeUuid = {
.name = afs_SRXCBProbeUuid_name,
.name = "CB.ProbeUuid",
.deliver = afs_deliver_cb_probe_uuid,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_ProbeUuid,
Expand All @@ -91,9 +82,8 @@ static const struct afs_call_type afs_SRXCBProbeUuid = {
/*
* CB.TellMeAboutYourself operation type
*/
static CM_NAME(TellMeAboutYourself);
static const struct afs_call_type afs_SRXCBTellMeAboutYourself = {
.name = afs_SRXCBTellMeAboutYourself_name,
.name = "CB.TellMeAboutYourself",
.deliver = afs_deliver_cb_tell_me_about_yourself,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_TellMeAboutYourself,
Expand All @@ -102,9 +92,8 @@ static const struct afs_call_type afs_SRXCBTellMeAboutYourself = {
/*
* YFS CB.CallBack operation type
*/
static CM_NAME(YFS_CallBack);
static const struct afs_call_type afs_SRXYFSCB_CallBack = {
.name = afs_SRXCBYFS_CallBack_name,
.name = "YFSCB.CallBack",
.deliver = afs_deliver_yfs_cb_callback,
.destructor = afs_cm_destructor,
.work = SRXAFSCB_CallBack,
Expand Down
67 changes: 62 additions & 5 deletions include/trace/events/afs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,34 @@ enum afs_vl_operation {
afs_VL_GetCapabilities = 65537, /* AFS Get VL server capabilities */
};

enum afs_cm_operation {
afs_CB_CallBack = 204, /* AFS break callback promises */
afs_CB_InitCallBackState = 205, /* AFS initialise callback state */
afs_CB_Probe = 206, /* AFS probe client */
afs_CB_GetLock = 207, /* AFS get contents of CM lock table */
afs_CB_GetCE = 208, /* AFS get cache file description */
afs_CB_GetXStatsVersion = 209, /* AFS get version of extended statistics */
afs_CB_GetXStats = 210, /* AFS get contents of extended statistics data */
afs_CB_InitCallBackState3 = 213, /* AFS initialise callback state, version 3 */
afs_CB_ProbeUuid = 214, /* AFS check the client hasn't rebooted */
};

enum yfs_cm_operation {
yfs_CB_Probe = 206, /* YFS probe client */
yfs_CB_GetLock = 207, /* YFS get contents of CM lock table */
yfs_CB_XStatsVersion = 209, /* YFS get version of extended statistics */
yfs_CB_GetXStats = 210, /* YFS get contents of extended statistics data */
yfs_CB_InitCallBackState3 = 213, /* YFS initialise callback state, version 3 */
yfs_CB_ProbeUuid = 214, /* YFS check the client hasn't rebooted */
yfs_CB_GetServerPrefs = 215,
yfs_CB_GetCellServDV = 216,
yfs_CB_GetLocalCell = 217,
yfs_CB_GetCacheConfig = 218,
yfs_CB_GetCellByNum = 65537,
yfs_CB_TellMeAboutYourself = 65538, /* get client capabilities */
yfs_CB_CallBack = 64204,
};

enum afs_edit_dir_op {
afs_edit_dir_create,
afs_edit_dir_create_error,
Expand Down Expand Up @@ -436,6 +464,32 @@ enum afs_cb_break_reason {
EM(afs_YFSVL_GetCellName, "YFSVL.GetCellName") \
E_(afs_VL_GetCapabilities, "VL.GetCapabilities")

#define afs_cm_operations \
EM(afs_CB_CallBack, "CB.CallBack") \
EM(afs_CB_InitCallBackState, "CB.InitCallBackState") \
EM(afs_CB_Probe, "CB.Probe") \
EM(afs_CB_GetLock, "CB.GetLock") \
EM(afs_CB_GetCE, "CB.GetCE") \
EM(afs_CB_GetXStatsVersion, "CB.GetXStatsVersion") \
EM(afs_CB_GetXStats, "CB.GetXStats") \
EM(afs_CB_InitCallBackState3, "CB.InitCallBackState3") \
E_(afs_CB_ProbeUuid, "CB.ProbeUuid")

#define yfs_cm_operations \
EM(yfs_CB_Probe, "YFSCB.Probe") \
EM(yfs_CB_GetLock, "YFSCB.GetLock") \
EM(yfs_CB_XStatsVersion, "YFSCB.XStatsVersion") \
EM(yfs_CB_GetXStats, "YFSCB.GetXStats") \
EM(yfs_CB_InitCallBackState3, "YFSCB.InitCallBackState3") \
EM(yfs_CB_ProbeUuid, "YFSCB.ProbeUuid") \
EM(yfs_CB_GetServerPrefs, "YFSCB.GetServerPrefs") \
EM(yfs_CB_GetCellServDV, "YFSCB.GetCellServDV") \
EM(yfs_CB_GetLocalCell, "YFSCB.GetLocalCell") \
EM(yfs_CB_GetCacheConfig, "YFSCB.GetCacheConfig") \
EM(yfs_CB_GetCellByNum, "YFSCB.GetCellByNum") \
EM(yfs_CB_TellMeAboutYourself, "YFSCB.TellMeAboutYourself") \
E_(yfs_CB_CallBack, "YFSCB.CallBack")

#define afs_edit_dir_ops \
EM(afs_edit_dir_create, "create") \
EM(afs_edit_dir_create_error, "c_fail") \
Expand Down Expand Up @@ -569,6 +623,8 @@ afs_server_traces;
afs_cell_traces;
afs_fs_operations;
afs_vl_operations;
afs_cm_operations;
yfs_cm_operations;
afs_edit_dir_ops;
afs_edit_dir_reasons;
afs_eproto_causes;
Expand Down Expand Up @@ -649,20 +705,21 @@ TRACE_EVENT(afs_cb_call,

TP_STRUCT__entry(
__field(unsigned int, call )
__field(const char *, name )
__field(u32, op )
__field(u16, service_id )
),

TP_fast_assign(
__entry->call = call->debug_id;
__entry->name = call->type->name;
__entry->op = call->operation_ID;
__entry->service_id = call->service_id;
),

TP_printk("c=%08x %s o=%u",
TP_printk("c=%08x %s",
__entry->call,
__entry->name,
__entry->op)
__entry->service_id == 2501 ?
__print_symbolic(__entry->op, yfs_cm_operations) :
__print_symbolic(__entry->op, afs_cm_operations))
);

TRACE_EVENT(afs_call,
Expand Down

0 comments on commit 6c881ca

Please sign in to comment.